Bugfix: Orderbook locks (#666)

engine/routines_test.go

@@ -81,16 +81,6 @@ func TestHandleData(t *testing.T) {
 		t.Error("Expected order to be modified to Active")
 	}
 
-	err = b.WebsocketDataHandler(exchName, &order.Cancel{
-		Exchange: fakePassExchange,
-		ID:       orderID,
-	})
-	if err != nil {
-		t.Error(err)
-	}
-	if origOrder.Status != order.Cancelled {
-		t.Error("Expected order status to be cancelled")
-	}
 	// Send some gibberish
 	err = b.WebsocketDataHandler(exchName, order.Stop)
 	if err != nil {

exchanges/binance/binance_websocket.go

@@ -544,6 +544,7 @@ func (b *Binance) Subscribe(channelsToSubscribe []stream.ChannelSubscription) er
 	for i := range channelsToSubscribe {
 		payload.Params = append(payload.Params, channelsToSubscribe[i].Channel)
 	}
+
 	err := b.Websocket.Conn.SendJSONMessage(payload)
 	if err != nil {
 		return err

exchanges/orderbook/calculator_test.go

@@ -1,6 +1,7 @@
 package orderbook
 
 import (
+	"errors"
 	"testing"
 
 	"github.com/thrasher-corp/gocryptotrader/currency"
@@ -32,14 +33,26 @@ func TestWhaleBomb(t *testing.T) {
 	}
 
 	// valid
-	b.WhaleBomb(7001, true)
+	_, err = b.WhaleBomb(7001, true)
+	if !errors.Is(err, nil) {
+		t.Errorf("received '%v', expected '%v'", err, nil)
+	}
 	// invalid
-	b.WhaleBomb(7002, true)
+	_, err = b.WhaleBomb(7002, true)
+	if err == nil {
+		t.Error("unexpected result")
+	}
 
 	// valid
-	b.WhaleBomb(6998, false)
+	_, err = b.WhaleBomb(6998, false)
+	if !errors.Is(err, nil) {
+		t.Errorf("received '%v', expected '%v'", err, nil)
+	}
 	// invalid
-	b.WhaleBomb(6997, false)
+	_, err = b.WhaleBomb(6997, false)
+	if err == nil {
+		t.Error("unexpected result")
+	}
 }
 
 func TestSimulateOrder(t *testing.T) {

exchanges/orderbook/node_test.go

@@ -9,9 +9,9 @@ import (
 
 func TestPushPop(t *testing.T) {
 	s := newStack()
-	var nSlice []*node
+	var nSlice [100]*node
 	for i := 0; i < 100; i++ {
-		nSlice = append(nSlice, s.Pop())
+		nSlice[i] = s.Pop()
 	}
 
 	if s.getCount() != 0 {
@@ -29,9 +29,9 @@ func TestPushPop(t *testing.T) {
 
 func TestCleaner(t *testing.T) {
 	s := newStack()
-	var nSlice []*node
+	var nSlice [100]*node
 	for i := 0; i < 100; i++ {
-		nSlice = append(nSlice, s.Pop())
+		nSlice[i] = s.Pop()
 	}
 
 	tn := getNow()
@@ -39,16 +39,16 @@ func TestCleaner(t *testing.T) {
 		s.Push(nSlice[i], tn)
 	}
 	// Makes all the 50 pushed nodes invalid
-	time.Sleep(time.Millisecond * 550)
+	time.Sleep(time.Millisecond * 260)
 	tn = getNow()
 	for i := 50; i < 100; i++ {
 		s.Push(nSlice[i], tn)
 	}
-	time.Sleep(time.Millisecond * 550)
+	time.Sleep(time.Millisecond * 50)
 	if s.getCount() != 50 {
 		t.Fatalf("incorrect stack count expected %v but received %v", 50, s.getCount())
 	}
-	time.Sleep(time.Second)
+	time.Sleep(time.Millisecond * 350)
 	if s.getCount() != 0 {
 		t.Fatalf("incorrect stack count expected %v but received %v", 0, s.getCount())
 	}

exchanges/orderbook/orderbook.go

@@ -98,6 +98,7 @@ func (s *Service) DeployDepth(exchange string, p currency.Pair, a asset.Item) (*
 		return nil, errAssetTypeNotSet
 	}
 	s.Lock()
+	defer s.Unlock()
 	m1, ok := s.books[strings.ToLower(exchange)]
 	if !ok {
 		id, err := s.Mux.GetID()
@@ -125,7 +126,6 @@ func (s *Service) DeployDepth(exchange string, p currency.Pair, a asset.Item) (*
 		book = newDepth(m1.ID)
 		m3[p.Quote.Item] = book
 	}
-	s.Unlock()
 	return book, nil
 }
 

exchanges/orderbook/orderbook_test.go

@@ -17,8 +17,8 @@ import (
 
 func TestMain(m *testing.M) {
 	// Sets up lower values for test environment
-	defaultInterval = time.Second
-	defaultAllowance = time.Millisecond * 500
+	defaultInterval = time.Millisecond * 250
+	defaultAllowance = time.Millisecond * 100
 	err := dispatch.Start(1, dispatch.DefaultJobsLimit)
 	if err != nil {
 		log.Fatal(err)
@@ -159,7 +159,7 @@ func TestCalculateTotalBids(t *testing.T) {
 	}
 }
 
-func TestCalculateTotaAsks(t *testing.T) {
+func TestCalculateTotalAsks(t *testing.T) {
 	t.Parallel()
 	curr, err := currency.NewPairFromStrings("BTC", "USD")
 	if err != nil {
@@ -204,14 +204,14 @@ func TestGetOrderbook(t *testing.T) {
 	}
 
 	_, err = Get("nonexistent", c, asset.Spot)
-	if err == nil {
-		t.Fatal("TestGetOrderbook retrieved non-existent orderbook")
+	if !errors.Is(err, errCannotFindOrderbook) {
+		t.Fatalf("received '%v', expected '%v'", err, errCannotFindOrderbook)
 	}
 
 	c.Base = currency.NewCode("blah")
 	_, err = Get("Exchange", c, asset.Spot)
-	if err == nil {
-		t.Fatal("TestGetOrderbook retrieved non-existent orderbook using invalid first currency")
+	if !errors.Is(err, errCannotFindOrderbook) {
+		t.Fatalf("received '%v', expected '%v', using invalid first currency", err, errCannotFindOrderbook)
 	}
 
 	newCurrency, err := currency.NewPairFromStrings("BTC", "AUD")
@@ -219,8 +219,8 @@ func TestGetOrderbook(t *testing.T) {
 		t.Fatal(err)
 	}
 	_, err = Get("Exchange", newCurrency, asset.Spot)
-	if err == nil {
-		t.Fatal("TestGetOrderbook retrieved non-existent orderbook using invalid second currency")
+	if !errors.Is(err, errCannotFindOrderbook) {
+		t.Fatalf("received '%v', expected '%v', using invalid second currency", err, errCannotFindOrderbook)
 	}
 
 	base.Pair = newCurrency
@@ -273,7 +273,7 @@ func TestGetDepth(t *testing.T) {
 		t.Fatalf("expecting %s error but received %v", errCannotFindOrderbook, err)
 	}
 
-	newCurrency, err := currency.NewPairFromStrings("BTC", "AUD")
+	newCurrency, err := currency.NewPairFromStrings("BTC", "DOGE")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -484,10 +484,12 @@ func TestProcessOrderbook(t *testing.T) {
 	var catastrophicFailure bool
 
 	for i := 0; i < 500; i++ {
+		m.Lock()
 		if catastrophicFailure {
+			m.Unlock()
 			break
 		}
-
+		m.Unlock()
 		wg.Add(1)
 		go func() {
 			newName := "Exchange" + strconv.FormatInt(rand.Int63(), 10) // nolint:gosec // no need to import crypo/rand for testing
@@ -509,36 +511,36 @@ func TestProcessOrderbook(t *testing.T) {
 			if err != nil {
 				t.Error(err)
 				catastrophicFailure = true
+				m.Unlock()
+				wg.Done()
 				return
 			}
-
 			testArray = append(testArray, quick{Name: newName, P: newPairs, Bids: bids, Asks: asks})
 			m.Unlock()
 			wg.Done()
 		}()
 	}
 
+	wg.Wait()
 	if catastrophicFailure {
 		t.Fatal("Process() error", err)
 	}
 
-	wg.Wait()
-
 	for _, test := range testArray {
 		wg.Add(1)
 		fatalErr := false
-		go func(test quick) {
-			result, err := Get(test.Name, test.P, asset.Spot)
+		go func(q quick) {
+			result, err := Get(q.Name, q.P, asset.Spot)
 			if err != nil {
 				fatalErr = true
 				return
 			}
 
-			if result.Asks[0] != test.Asks[0] {
+			if result.Asks[0] != q.Asks[0] {
 				t.Error("TestProcessOrderbook failed bad values")
 			}
 
-			if result.Bids[0] != test.Bids[0] {
+			if result.Bids[0] != q.Bids[0] {
 				t.Error("TestProcessOrderbook failed bad values")
 			}