Merge pull request #4706 from freqtrade/simplify_fiat_convert

Simplify fiat convert and fix USD coingecko problem

freqtrade/rpc/fiat_convert.py

@@ -4,9 +4,9 @@ e.g BTC to USD
 """
 
 import logging
-import time
+from typing import Dict
-from typing import Dict, List
 
+from cachetools.ttl import TTLCache
 from pycoingecko import CoinGeckoAPI
 
 from freqtrade.constants import SUPPORTED_FIAT
@@ -15,51 +15,6 @@ from freqtrade.constants import SUPPORTED_FIAT
 logger = logging.getLogger(__name__)
 
 
-class CryptoFiat:
-    """
-    Object to describe what is the price of Crypto-currency in a FIAT
-    """
-    # Constants
-    CACHE_DURATION = 6 * 60 * 60  # 6 hours
-
-    def __init__(self, crypto_symbol: str, fiat_symbol: str, price: float) -> None:
-        """
-        Create an object that will contains the price for a crypto-currency in fiat
-        :param crypto_symbol: Crypto-currency you want to convert (e.g BTC)
-        :param fiat_symbol: FIAT currency you want to convert to (e.g USD)
-        :param price: Price in FIAT
-        """
-
-        # Public attributes
-        self.crypto_symbol = None
-        self.fiat_symbol = None
-        self.price = 0.0
-
-        # Private attributes
-        self._expiration = 0.0
-
-        self.crypto_symbol = crypto_symbol.lower()
-        self.fiat_symbol = fiat_symbol.lower()
-        self.set_price(price=price)
-
-    def set_price(self, price: float) -> None:
-        """
-        Set the price of the Crypto-currency in FIAT and set the expiration time
-        :param price: Price of the current Crypto currency in the fiat
-        :return: None
-        """
-        self.price = price
-        self._expiration = time.time() + self.CACHE_DURATION
-
-    def is_expired(self) -> bool:
-        """
-        Return if the current price is still valid or needs to be refreshed
-        :return: bool, true the price is expired and needs to be refreshed, false the price is
-         still valid
-        """
-        return self._expiration - time.time() <= 0
-
-
 class CryptoToFiatConverter:
     """
     Main class to initiate Crypto to FIAT.
@@ -84,7 +39,9 @@ class CryptoToFiatConverter:
         return CryptoToFiatConverter.__instance
 
     def __init__(self) -> None:
-        self._pairs: List[CryptoFiat] = []
+        # Timeout: 6h
+        self._pair_price: TTLCache = TTLCache(maxsize=500, ttl=6 * 60 * 60)
+
         self._load_cryptomap()
 
     def _load_cryptomap(self) -> None:
@@ -118,49 +75,31 @@ class CryptoToFiatConverter:
         """
         crypto_symbol = crypto_symbol.lower()
         fiat_symbol = fiat_symbol.lower()
+        inverse = False
 
+        if crypto_symbol == 'usd':
+            # usd corresponds to "uniswap-state-dollar" for coingecko.
+            # We'll therefore need to "swap" the currencies
+            logger.info(f"reversing Rates {crypto_symbol}, {fiat_symbol}")
+            crypto_symbol = fiat_symbol
+            fiat_symbol = 'usd'
+            inverse = True
+
+        symbol = f"{crypto_symbol}/{fiat_symbol}"
         # Check if the fiat convertion you want is supported
         if not self._is_supported_fiat(fiat=fiat_symbol):
             raise ValueError(f'The fiat {fiat_symbol} is not supported.')
 
-        # Get the pair that interest us and return the price in fiat
+        price = self._pair_price.get(symbol, None)
-        for pair in self._pairs:
-            if pair.crypto_symbol == crypto_symbol and pair.fiat_symbol == fiat_symbol:
-                # If the price is expired we refresh it, avoid to call the API all the time
-                if pair.is_expired():
-                    pair.set_price(
-                        price=self._find_price(
-                            crypto_symbol=pair.crypto_symbol,
-                            fiat_symbol=pair.fiat_symbol
-                        )
-                    )
 
-                # return the last price we have for this pair
+        if not price:
-                return pair.price
+            price = self._find_price(
-
-        # The pair does not exist, so we create it and return the price
-        return self._add_pair(
-            crypto_symbol=crypto_symbol,
-            fiat_symbol=fiat_symbol,
-            price=self._find_price(
                 crypto_symbol=crypto_symbol,
                 fiat_symbol=fiat_symbol
             )
-        )
+            if inverse and price != 0.0:
-
+                price = 1 / price
-    def _add_pair(self, crypto_symbol: str, fiat_symbol: str, price: float) -> float:
+            self._pair_price[symbol] = price
-        """
-        :param crypto_symbol: Crypto-currency you want to convert (e.g BTC)
-        :param fiat_symbol: FIAT currency you want to convert to (e.g USD)
-        :return: price in FIAT
-        """
-        self._pairs.append(
-            CryptoFiat(
-                crypto_symbol=crypto_symbol,
-                fiat_symbol=fiat_symbol,
-                price=price
-            )
-        )
 
         return price
 

tests/rpc/test_fiat_convert.py

@@ -1,44 +1,15 @@
 # pragma pylint: disable=missing-docstring, too-many-arguments, too-many-ancestors,
 # pragma pylint: disable=protected-access, C0103
 
-import time
 from unittest.mock import MagicMock
 
 import pytest
 from requests.exceptions import RequestException
 
-from freqtrade.rpc.fiat_convert import CryptoFiat, CryptoToFiatConverter
+from freqtrade.rpc.fiat_convert import CryptoToFiatConverter
 from tests.conftest import log_has, log_has_re
 
 
-def test_pair_convertion_object():
-    pair_convertion = CryptoFiat(
-        crypto_symbol='btc',
-        fiat_symbol='usd',
-        price=12345.0
-    )
-
-    # Check the cache duration is 6 hours
-    assert pair_convertion.CACHE_DURATION == 6 * 60 * 60
-
-    # Check a regular usage
-    assert pair_convertion.crypto_symbol == 'btc'
-    assert pair_convertion.fiat_symbol == 'usd'
-    assert pair_convertion.price == 12345.0
-    assert pair_convertion.is_expired() is False
-
-    # Update the expiration time (- 2 hours) and check the behavior
-    pair_convertion._expiration = time.time() - 2 * 60 * 60
-    assert pair_convertion.is_expired() is True
-
-    # Check set price behaviour
-    time_reference = time.time() + pair_convertion.CACHE_DURATION
-    pair_convertion.set_price(price=30000.123)
-    assert pair_convertion.is_expired() is False
-    assert pair_convertion._expiration >= time_reference
-    assert pair_convertion.price == 30000.123
-
-
 def test_fiat_convert_is_supported(mocker):
     fiat_convert = CryptoToFiatConverter()
     assert fiat_convert._is_supported_fiat(fiat='USD') is True
@@ -47,28 +18,6 @@ def test_fiat_convert_is_supported(mocker):
     assert fiat_convert._is_supported_fiat(fiat='ABC') is False
 
 
-def test_fiat_convert_add_pair(mocker):
-
-    fiat_convert = CryptoToFiatConverter()
-
-    pair_len = len(fiat_convert._pairs)
-    assert pair_len == 0
-
-    fiat_convert._add_pair(crypto_symbol='btc', fiat_symbol='usd', price=12345.0)
-    pair_len = len(fiat_convert._pairs)
-    assert pair_len == 1
-    assert fiat_convert._pairs[0].crypto_symbol == 'btc'
-    assert fiat_convert._pairs[0].fiat_symbol == 'usd'
-    assert fiat_convert._pairs[0].price == 12345.0
-
-    fiat_convert._add_pair(crypto_symbol='btc', fiat_symbol='Eur', price=13000.2)
-    pair_len = len(fiat_convert._pairs)
-    assert pair_len == 2
-    assert fiat_convert._pairs[1].crypto_symbol == 'btc'
-    assert fiat_convert._pairs[1].fiat_symbol == 'eur'
-    assert fiat_convert._pairs[1].price == 13000.2
-
-
 def test_fiat_convert_find_price(mocker):
     fiat_convert = CryptoToFiatConverter()
 
@@ -95,7 +44,7 @@ def test_fiat_convert_unsupported_crypto(mocker, caplog):
 
 
 def test_fiat_convert_get_price(mocker):
-    mocker.patch('freqtrade.rpc.fiat_convert.CryptoToFiatConverter._find_price',
+    find_price = mocker.patch('freqtrade.rpc.fiat_convert.CryptoToFiatConverter._find_price',
                               return_value=28000.0)
 
     fiat_convert = CryptoToFiatConverter()
@@ -104,26 +53,17 @@ def test_fiat_convert_get_price(mocker):
         fiat_convert.get_price(crypto_symbol='btc', fiat_symbol='US Dollar')
 
     # Check the value return by the method
-    pair_len = len(fiat_convert._pairs)
+    pair_len = len(fiat_convert._pair_price)
     assert pair_len == 0
     assert fiat_convert.get_price(crypto_symbol='btc', fiat_symbol='usd') == 28000.0
-    assert fiat_convert._pairs[0].crypto_symbol == 'btc'
+    assert fiat_convert._pair_price['btc/usd'] == 28000.0
-    assert fiat_convert._pairs[0].fiat_symbol == 'usd'
+    assert len(fiat_convert._pair_price) == 1
-    assert fiat_convert._pairs[0].price == 28000.0
+    assert find_price.call_count == 1
-    assert fiat_convert._pairs[0]._expiration != 0
-    assert len(fiat_convert._pairs) == 1
 
     # Verify the cached is used
-    fiat_convert._pairs[0].price = 9867.543
+    fiat_convert._pair_price['btc/usd'] = 9867.543
-    expiration = fiat_convert._pairs[0]._expiration
     assert fiat_convert.get_price(crypto_symbol='btc', fiat_symbol='usd') == 9867.543
-    assert fiat_convert._pairs[0]._expiration == expiration
+    assert find_price.call_count == 1
-
-    # Verify the cache expiration
-    expiration = time.time() - 2 * 60 * 60
-    fiat_convert._pairs[0]._expiration = expiration
-    assert fiat_convert.get_price(crypto_symbol='btc', fiat_symbol='usd') == 28000.0
-    assert fiat_convert._pairs[0]._expiration is not expiration
 
 
 def test_fiat_convert_same_currencies(mocker):