fix(fiat-rates): if historical rate missing for given date, return -1

common/metrics.go

@@ -95,6 +95,7 @@ type Metrics struct {
 	FiatRatesFetchedUnits             *prometheus.CounterVec   `metric:"fiat_rates_fetched_units_total"`
 	FiatRatesFetchedTokens            *prometheus.CounterVec   `metric:"fiat_rates_fetched_tokens_total"`
 	FiatRatesUnable                   *prometheus.CounterVec   `metric:"fiat_rates_unable_total"`
+	FiatRatesMissingDayLookups        prometheus.Counter       `metric:"fiat_rates_missing_day_lookups"`
 	AlternativeFeeProviderRequests    *prometheus.CounterVec   `metric:"alternative_fee_provider_requests"`
 	EthSyncRpcErrors                  *prometheus.CounterVec   `metric:"eth_sync_rpc_errors"`
 }

configs/metrics.yaml

@@ -368,6 +368,10 @@ metrics:
     type: counter_vec
     help: Fiat-rate series that could not be fetched, split by phase and reason (gap_too_large, fetch_failed, provider_banned, no_base_ticker, budget_exhausted)
     labels: [phase, reason]
+  fiat_rates_missing_day_lookups:
+    name: blockbook_fiat_rates_missing_day_lookups
+    type: counter
+    help: Fiat rate lookups that found no stored historical ticker for the resolved day and returned an unavailable rate
   alternative_fee_provider_requests:
     name: blockbook_alternative_fee_provider_requests
     type: counter_vec

fiat/fiat_rates.go

@@ -283,7 +283,6 @@ func (fr *FiatRates) GetTickersForTimestamps(timestamps []int64, vsCurrency stri
 	hourlyTickersFrom := fr.hourlyTickersFrom
 	hourlyTickersTo := fr.hourlyTickersTo
 	dailyTickers := fr.dailyTickers
-	dailyTickersFrom := fr.dailyTickersFrom
 	dailyTickersTo := fr.dailyTickersTo
 	fr.mux.RUnlock()
 
@@ -315,28 +314,24 @@ func (fr *FiatRates) GetTickersForTimestamps(timestamps []int64, vsCurrency stri
 			tickers[i] = prevTicker
 			continue
 		} else {
-			var found bool
-			if dailyTs < dailyTickersFrom {
-				dailyTs = dailyTickersFrom
-			}
-			var ticker *common.CurrencyRatesTicker
-			for ; dailyTs <= dailyTickersTo; dailyTs += secondsInDay {
-				if ticker, found = dailyTickers[dailyTs]; found && ticker != nil {
-					if common.IsSuitableTicker(ticker, vsCurrency, token) {
-						tickers[i] = ticker
-						prevTicker = ticker
-						prevTs = t
-						break
-					} else {
-						found = false
-					}
+			if dailyTs <= dailyTickersTo {
+				// serve only a ticker stored for the resolved day, otherwise keep nil
+				// so that the API returns -1
+				if ticker, found := dailyTickers[dailyTs]; found && ticker != nil &&
+					roundTimeUnix(ticker.Timestamp, secondsInDay) == dailyTs &&
+					common.IsSuitableTicker(ticker, vsCurrency, token) {
+					tickers[i] = ticker
+					prevTicker = ticker
+					prevTs = t
+				} else if fr.metrics != nil {
+					fr.metrics.FiatRatesMissingDayLookups.Inc()
 				}
+				continue
 			}
-			if !found {
-				tickers[i] = currentTicker
-				prevTicker = currentTicker
-				prevTs = t
-			}
+			// timestamps after the stored history are served with the current ticker
+			tickers[i] = currentTicker
+			prevTicker = currentTicker
+			prevTs = t
 		}
 	}
 	return &tickers, nil

fiat/fiat_rates_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/golang/glog"
 	"github.com/linxGnu/grocksdb"
 	"github.com/martinboehm/btcutil/chaincfg"
+	dto "github.com/prometheus/client_model/go"
 	"github.com/trezor/blockbook/bchain"
 	"github.com/trezor/blockbook/bchain/coins/btc"
 	"github.com/trezor/blockbook/common"
@@ -420,6 +421,100 @@ func TestGetTickersForTimestamps_UsesGranularityAndFallback(t *testing.T) {
 	}
 }
 
+func TestGetTickersForTimestamps_GapInHistoryReturnsNil(t *testing.T) {
+	const day = int64(secondsInDay)
+	day1 := &common.CurrencyRatesTicker{Timestamp: time.Unix(day, 0).UTC(), Rates: map[string]float32{"usd": 1, "czk": 10}}
+	day2 := &common.CurrencyRatesTicker{Timestamp: time.Unix(2*day, 0).UTC(), Rates: map[string]float32{"usd": 2, "czk": 20}}
+	// czk is missing since day3, the same shape as a partially failing historical update
+	day3 := &common.CurrencyRatesTicker{Timestamp: time.Unix(3*day, 0).UTC(), Rates: map[string]float32{"usd": 3}}
+	day4 := &common.CurrencyRatesTicker{Timestamp: time.Unix(4*day, 0).UTC(), Rates: map[string]float32{"usd": 4}}
+	currentTicker := &common.CurrencyRatesTicker{Timestamp: time.Unix(5*day, 0).UTC(), Rates: map[string]float32{"usd": 5, "czk": 50}}
+	metrics, err := common.GetMetrics("fakecoin")
+	if err != nil {
+		t.Fatalf("GetMetrics failed: %v", err)
+	}
+	fr := &FiatRates{
+		Enabled:       true,
+		metrics:       metrics,
+		currentTicker: currentTicker,
+		dailyTickers: map[int64]*common.CurrencyRatesTicker{
+			day:     day1,
+			2 * day: day2,
+			3 * day: day3,
+			4 * day: day4,
+		},
+		dailyTickersFrom: day,
+		dailyTickersTo:   4 * day,
+	}
+
+	tickers, err := fr.GetTickersForTimestamps([]int64{0, 50000, 100000, 200000, 350000}, "czk", "")
+	if err != nil {
+		t.Fatalf("GetTickersForTimestamps returned error: %v", err)
+	}
+	want := []*common.CurrencyRatesTicker{
+		nil,           // before the stored history, there is no rate for the requested day, the API returns -1
+		day1,          // resolved to the first stored day
+		day2,          // within the stored history, the ticker of the requested day is returned
+		nil,           // czk missing on the requested day, nil makes the API return -1
+		currentTicker, // beyond the stored history, the current ticker is the closest available rate
+	}
+	if tickers == nil || !reflect.DeepEqual(*tickers, want) {
+		t.Fatalf("unexpected tickers: got %+v, want %+v", tickers, want)
+	}
+
+	// a currency never present in the stored history resolves to nil within the historical range
+	tickers, err = fr.GetTickersForTimestamps([]int64{100000}, "xyz", "")
+	if err != nil {
+		t.Fatalf("GetTickersForTimestamps returned error: %v", err)
+	}
+	if tickers == nil || len(*tickers) != 1 || (*tickers)[0] != nil {
+		t.Fatalf("unexpected tickers for unknown currency: got %+v, want [nil]", tickers)
+	}
+
+	// other currencies within the gap days are not affected
+	tickers, err = fr.GetTickersForTimestamps([]int64{200000}, "usd", "")
+	if err != nil {
+		t.Fatalf("GetTickersForTimestamps returned error: %v", err)
+	}
+	if tickers == nil || len(*tickers) != 1 || (*tickers)[0] != day3 {
+		t.Fatalf("unexpected tickers for usd: got %+v, want day3 ticker", tickers)
+	}
+
+	// a day missing in the DB is filled with the next day ticker by loadDailyTickers;
+	// such day must be reported as unavailable even though the filling ticker contains
+	// the requested currency
+	fr.dailyTickers = map[int64]*common.CurrencyRatesTicker{
+		day:     day1,
+		2 * day: day3, // forward-fill of the missing day2
+		3 * day: day3,
+		4 * day: day4,
+	}
+	tickers, err = fr.GetTickersForTimestamps([]int64{100000}, "usd", "")
+	if err != nil {
+		t.Fatalf("GetTickersForTimestamps returned error: %v", err)
+	}
+	if tickers == nil || len(*tickers) != 1 || (*tickers)[0] != nil {
+		t.Fatalf("unexpected tickers for a missing day: got %+v, want [nil]", tickers)
+	}
+	// the next day with a stored ticker is still served normally
+	tickers, err = fr.GetTickersForTimestamps([]int64{200000}, "usd", "")
+	if err != nil {
+		t.Fatalf("GetTickersForTimestamps returned error: %v", err)
+	}
+	if tickers == nil || len(*tickers) != 1 || (*tickers)[0] != day3 {
+		t.Fatalf("unexpected tickers for the day after a missing day: got %+v, want day3 ticker", tickers)
+	}
+
+	// every nil result above must be counted by the missing day lookups metric
+	var m dto.Metric
+	if err := metrics.FiatRatesMissingDayLookups.Write(&m); err != nil {
+		t.Fatalf("counter Write failed: %v", err)
+	}
+	if got := m.GetCounter().GetValue(); got != 4 {
+		t.Fatalf("unexpected missing day lookups count: got %v, want 4", got)
+	}
+}
+
 func TestGetTickersForTimestamps_ConcurrentReadersAndWriters(t *testing.T) {
 	fr := &FiatRates{Enabled: true}
 

openapi.yaml

@@ -772,11 +772,13 @@ paths:
       operationId: getFiatTicker
       summary: Get current or historical fiat rates.
       description: |-
-        Returns currency rates for the requested currency and date. If a rate
-        is unavailable for the exact timestamp, the closest available rate can
-        be returned. Responses include the actual rate timestamp. Without a
-        currency parameter, all available currencies can be returned. A rate of
-        -1 marks an unavailable or invalid currency for that timestamp.
+        Returns currency rates for the requested currency and date. Rates are
+        resolved forward to the nearest stored rate at or after the requested
+        timestamp within the same UTC day; responses include the actual rate
+        timestamp. A rate of -1 marks an invalid currency or a day with no
+        stored rate. Timestamps newer than the stored history return the
+        current rate. Without a currency parameter, all available currencies
+        can be returned.
 
         Load estimate: Low to medium; specific currency lookups are cheap,
         while omitted currency and token lookups increase response size.

server/public_test.go

@@ -678,12 +678,12 @@ func httpTestsBitcoinType(t *testing.T, ts *httptest.Server) {
 			},
 		},
 		{
-			name:        "apiMultiFiatRates all currencies",
+			name:        "apiMultiFiatRates all currencies, the second timestamp resolves to a missing day",
 			r:           newGetRequest(ts.URL + "/api/v2/multi-tickers?timestamp=1574344800,1521677000"),
 			status:      http.StatusOK,
 			contentType: "application/json; charset=utf-8",
 			body: []string{
-				`[{"ts":1574380800,"rates":{"eur":7134.1,"usd":7914.5}},{"ts":1521849600,"rates":{"eur":1303,"usd":2003}}]`,
+				`[{"ts":1574380800,"rates":{"eur":7134.1,"usd":7914.5}},{"ts":1521677000,"rates":{}}]`,
 			},
 		},
 		{
@@ -696,12 +696,12 @@ func httpTestsBitcoinType(t *testing.T, ts *httptest.Server) {
 			},
 		},
 		{
-			name:        "apiFiatRates get closest rate",
+			name:        "apiFiatRates timestamp before stored history",
 			r:           newGetRequest(ts.URL + "/api/v2/tickers?timestamp=1357045200&currency=usd"),
 			status:      http.StatusOK,
 			contentType: "application/json; charset=utf-8",
 			body: []string{
-				`{"ts":1521504000,"rates":{"usd":2000}}`,
+				`{"ts":1357045200,"rates":{"usd":-1}}`,
 			},
 		},
 		{
@@ -1536,26 +1536,26 @@ var websocketTestsBitcoinType = []websocketTest{
 		want: `{"id":"27","data":{"tickers":[{"ts":1521590400,"rates":{"eur":1301}}]}}`,
 	},
 	{
-		name: "websocket getFiatRatesForTimestamps multiple timestamps usd",
+		name: "websocket getFiatRatesForTimestamps multiple timestamps usd, the first resolves to a missing day",
 		req: websocketReq{
 			Method: "getFiatRatesForTimestamps",
 			Params: map[string]interface{}{
 				"currencies": []string{"usd"},
 				"timestamps": []int64{1570346615, 1574346615},
 			},
 		},
-		want: `{"id":"28","data":{"tickers":[{"ts":1574294400,"rates":{"usd":7814.5}},{"ts":1574380800,"rates":{"usd":7914.5}}]}}`,
+		want: `{"id":"28","data":{"tickers":[{"ts":1570346615,"rates":{"usd":-1}},{"ts":1574380800,"rates":{"usd":7914.5}}]}}`,
 	},
 	{
-		name: "websocket getFiatRatesForTimestamps multiple timestamps eur",
+		name: "websocket getFiatRatesForTimestamps multiple timestamps eur, the first resolves to a missing day",
 		req: websocketReq{
 			Method: "getFiatRatesForTimestamps",
 			Params: map[string]interface{}{
 				"currencies": []string{"eur"},
 				"timestamps": []int64{1570346615, 1574346615},
 			},
 		},
-		want: `{"id":"29","data":{"tickers":[{"ts":1574294400,"rates":{"eur":7100}},{"ts":1574380800,"rates":{"eur":7134.1}}]}}`,
+		want: `{"id":"29","data":{"tickers":[{"ts":1570346615,"rates":{"eur":-1}},{"ts":1574380800,"rates":{"eur":7134.1}}]}}`,
 	},
 	{
 		name: "websocket getFiatRatesForTimestamps multiple timestamps with an error",
@@ -1566,7 +1566,7 @@ var websocketTestsBitcoinType = []websocketTest{
 				"timestamps": []int64{1570346615, 1574346615, 2000000000},
 			},
 		},
-		want: `{"id":"30","data":{"tickers":[{"ts":1574294400,"rates":{"usd":7814.5}},{"ts":1574380800,"rates":{"usd":7914.5}},{"ts":2000000000,"rates":{"usd":-1}}]}}`,
+		want: `{"id":"30","data":{"tickers":[{"ts":1570346615,"rates":{"usd":-1}},{"ts":1574380800,"rates":{"usd":7914.5}},{"ts":2000000000,"rates":{"usd":-1}}]}}`,
 	},
 	{
 		name: "websocket getFiatRatesForTimestamps multiple errors",
@@ -1584,10 +1584,10 @@ var websocketTestsBitcoinType = []websocketTest{
 		req: websocketReq{
 			Method: "getFiatRatesTickersList",
 			Params: map[string]interface{}{
-				"timestamp": 1570346615,
+				"timestamp": 1574346615,
 			},
 		},
-		want: `{"id":"32","data":{"ts":1574294400,"available_currencies":["eur","usd"]}}`,
+		want: `{"id":"32","data":{"ts":1574380800,"available_currencies":["eur","usd"]}}`,
 	},
 	{
 		name: "websocket getBalanceHistory Addr2",
@@ -2038,7 +2038,8 @@ func Test_HTTPBalanceHistory_GroupByAndInvalidCurrency_BitcoinType(t *testing.T)
 			Received:   "18876",
 			Sent:       "9876",
 			SentToSelf: "9000",
-			Rates:      map[string]float32{"eur": 1300},
+			// the grouped bucket time resolves to a day before the first stored ticker, no rates are returned
+			Rates: nil,
 		},
 	}
 	if !reflect.DeepEqual(grouped, wantGrouped) {