From e866c8f08d8967247b8266eb9bc418fe313b3b67 Mon Sep 17 00:00:00 2001
From: Gareth Kirwan <gbjkirwan@gmail.com>
Date: Thu, 28 Dec 2023 05:41:41 +0100
Subject: [PATCH] Bitfinex: Fix UpdateTickers (#1427)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Bitfinex: Fix TestUpdateTickers on ICE

Accept 5% failure rate of available pairs not working as tickers.

When Bitfinex [delisted ICE](https://www.bitfinex.com/posts/990) it's still coming back in all pub:conf and pub:info listings:
```
❯ curl -s https://api.bitfinex.com/v2/conf/pub:info:pair | jq -c '.[0][] | select(.[0] | test("(BTC|ICE|ZIL)USD")) '                                                                   ["BTCUSD",[null,null,null,"0.00006","2000.0",null,null,null,0.1,0.05,null,null]]
["ICEUSD",[null,null,null,"4.0","25000.0",null,null,null,null,null,null,null]]
["ZILUSD",[null,null,null,"40.0","1500000.0",null,null,null,null,null,null,null]]
```
_( I included ZIL to show a tradable pair without Margin fields )_
There's absolutely no sign it's not a tradable pair _until_ you ask for a ticker for it:
```
❯ curl -s https://api.bitfinex.com/v2/ticker/tICEUSD                                                                                                                                   ["error",10020,"symbol: invalid"]⏎
❯ curl -s https://api.bitfinex.com/v2/ticker/tBTCUSD                                                                                                                                   [42854,11.8920918,42855,12.71095442,-290,-0.00672292,42846,725.08132142,43288,41850]⏎
```

* fixup! Bitfinex: Fix TestUpdateTickers on ICE

* fixup! Bitfinex: Fix TestUpdateTickers on ICE

* Bitfinex: Fix UpdateTickers stopping on first error
---
 exchanges/bitfinex/bitfinex_test.go    | 57 +++++++++++++++-----------
 exchanges/bitfinex/bitfinex_wrapper.go | 12 +++---
 2 files changed, 39 insertions(+), 30 deletions(-)

diff --git a/exchanges/bitfinex/bitfinex_test.go b/exchanges/bitfinex/bitfinex_test.go
index 8e1ac1b3..064341ee 100644
--- a/exchanges/bitfinex/bitfinex_test.go
+++ b/exchanges/bitfinex/bitfinex_test.go
@@ -164,9 +164,17 @@ func TestGetPairs(t *testing.T) {
 
 func TestUpdateTradablePairs(t *testing.T) {
 	t.Parallel()
-	if err := b.UpdateTradablePairs(context.Background(), true); err != nil {
-		t.Error("Bitfinex UpdateTradablePairs() error", err)
-	}
+	updatePairsOnce(t)
+}
+
+var updatePairsGuard sync.Once
+
+func updatePairsOnce(tb testing.TB) {
+	tb.Helper()
+	updatePairsGuard.Do(func() {
+		err := b.UpdateTradablePairs(context.Background(), true)
+		assert.NoError(tb, err, "UpdateTradablePairs should not error")
+	})
 }
 
 func TestUpdateOrderExecutionLimits(t *testing.T) {
@@ -477,35 +485,34 @@ func TestUpdateTicker(t *testing.T) {
 func TestUpdateTickers(t *testing.T) {
 	t.Parallel()
 
-	err := b.UpdateTradablePairs(context.Background(), false)
-	if err != nil {
-		t.Fatal(err)
-	}
+	updatePairsOnce(t)
 
 	assets := b.GetAssetTypes(false)
-	for x := range assets {
-		var avail []currency.Pair
-		avail, err = b.GetAvailablePairs(assets[x])
-		if err != nil {
-			t.Fatal(err)
-		}
+	for _, a := range assets {
+		avail, err := b.GetAvailablePairs(a)
+		assert.NoError(t, err, "GetAvailablePairs should not error")
 
-		err = b.CurrencyPairs.StorePairs(assets[x], avail, true)
-		if err != nil {
-			t.Fatal(err)
-		}
+		err = b.CurrencyPairs.StorePairs(a, avail, true)
+		assert.NoError(t, err, "StorePairs should not error")
 
-		err = b.UpdateTickers(context.Background(), assets[x])
-		if err != nil {
-			t.Fatal(err)
-		}
+		err = b.UpdateTickers(context.Background(), a)
+		assert.NoError(t, common.ExcludeError(err, ticker.ErrBidEqualsAsk), "UpdateTickers may only error about locked markets")
 
-		for y := range avail {
-			_, err = ticker.GetTicker(b.Name, avail[y], assets[x])
-			if err != nil {
-				t.Fatal(err)
+		// Bitfinex leaves delisted pairs in Available info/conf endpoints
+		// We want to assert that most pairs are valid, so we'll check that no more than 5% are erroring
+		acceptableThreshold := 95.0
+		okay := 0.0
+		var errs error
+		for _, p := range avail {
+			if _, err = ticker.GetTicker(b.Name, p, a); err != nil {
+				errs = common.AppendError(errs, err)
+			} else {
+				okay++
 			}
 		}
+		if !assert.Greater(t, okay/float64(len(avail))*100.0, acceptableThreshold, "At least %.f%% of %s tickers should not error", acceptableThreshold, a) {
+			t.Log(errs.Error())
+		}
 	}
 }
 
diff --git a/exchanges/bitfinex/bitfinex_wrapper.go b/exchanges/bitfinex/bitfinex_wrapper.go
index 28a4d761..81cbdecc 100644
--- a/exchanges/bitfinex/bitfinex_wrapper.go
+++ b/exchanges/bitfinex/bitfinex_wrapper.go
@@ -366,15 +366,17 @@ func (b *Bitfinex) UpdateOrderExecutionLimits(ctx context.Context, a asset.Item)
 
 // UpdateTickers updates the ticker for all currency pairs of a given asset type
 func (b *Bitfinex) UpdateTickers(ctx context.Context, a asset.Item) error {
-	tickerNew, err := b.GetTickerBatch(ctx)
+	t, err := b.GetTickerBatch(ctx)
 	if err != nil {
 		return err
 	}
 
-	for key, val := range tickerNew {
+	var errs error
+	for key, val := range t {
 		pair, enabled, err := b.MatchSymbolCheckEnabled(key[1:], a, true)
 		if err != nil && !errors.Is(err, currency.ErrPairNotFound) {
-			return err
+			errs = common.AppendError(errs, err)
+			continue
 		}
 		if !enabled {
 			continue
@@ -391,10 +393,10 @@ func (b *Bitfinex) UpdateTickers(ctx context.Context, a asset.Item) error {
 			AssetType:    a,
 			ExchangeName: b.Name})
 		if err != nil {
-			return err
+			errs = common.AppendError(errs, err)
 		}
 	}
-	return nil
+	return errs
 }
 
 // UpdateTicker updates and returns the ticker for a currency pair