search

eprbell / dali-rp2

DaLI (Data Loader Interface) is a data loader and input generator for RP2 (https://pypi.org/project/rp2), the privacy-focused, free, open-source cryptocurrency tax calculator: DaLI removes the need to manually prepare RP2 input files. Just like RP2, DaLI is also free, open-source and it prioritizes user privacy.
https://pypi.org/project/dali-rp2/
Apache License 2.0
66 stars 42 forks source link

Spot price for XXX->YYY not found on any pair converter plugin #159

Closed topherbuckley closed 1 year ago

topherbuckley commented 1 year ago

TLDR

Gate.io doesn't support BOBA/USD direct conversion, but it looks like this is manually added to the ccxt.py. |

All the details:

Getting the following error when running the bitbank rest api

ERROR: Fatal exception occurred:
Traceback (most recent call last):
  File "/media/christopher/HDD/git/dali-rp2/src/dali/dali_main.py", line 181, in _dali_main_internal
    resolved_transactions: List[AbstractTransaction] = resolve_transactions(transactions, dali_configuration, args.read_spot_price_from_web)
  File "/media/christopher/HDD/git/dali-rp2/src/dali/transaction_resolver.py", line 263, in resolve_transactions
    transaction = _update_spot_price_from_web(transaction, global_configuration)
  File "/media/christopher/HDD/git/dali-rp2/src/dali/transaction_resolver.py", line 144, in _update_spot_price_from_web
    raise RP2RuntimeError(
rp2.rp2_error.RP2RuntimeError: Spot price for 1224573781:2022-05-10 06:20:39+00:00:BOBA->USD not found on any pair converter plugin

For context, the lines around transaction_resolver.py (135-148) are as follows:

    if is_unknown(transaction.spot_price) or RP2Decimal(transaction.spot_price) == ZERO:  # type: ignore
        conversion: RateAndPairConverter = _get_pair_conversion_rate(
            timestamp=transaction.timestamp_value,
            from_asset=transaction.asset,
            to_asset=global_configuration[Keyword.NATIVE_FIAT.value],
            exchange=_get_originating_exchange(transaction),
            global_configuration=global_configuration,
        )
        if conversion.rate is None:
            raise RP2RuntimeError(
                f"Spot price for {transaction.unique_id + ':' if transaction.unique_id else ''}"
                f"{transaction.timestamp_value}:{transaction.asset}->{global_configuration[Keyword.NATIVE_FIAT.value]}"
                " not found on any pair converter plugin"
            )

Jumping into _get_pair_conversion_rate:

def _get_pair_conversion_rate(timestamp: datetime, from_asset: str, to_asset: str, exchange: str, global_configuration: Dict[str, Any]) -> RateAndPairConverter:
    rate: Optional[RP2Decimal] = None
    pair_converter: Optional[AbstractPairConverterPlugin] = None
    for pair_converter in global_configuration[Keyword.HISTORICAL_PAIR_CONVERTERS.value]:
        rate = cast(AbstractPairConverterPlugin, pair_converter).get_conversion_rate(timestamp, from_asset, to_asset, exchange)
        if rate:
            break

    if pair_converter is None:
        raise RP2RuntimeError("No pair converter plugin found")

    return RateAndPairConverter(rate, pair_converter)

Here, pair_converter starts as an dali.plugin.pair_converter.historic_crypto.PairConverterPlugin, then moves on to be a dali.plugin.pair_converter.ccxt.PairConverterPlugin. Running along, with the historic_crypto.PairConverterPlugin as the pair_converter' and stepping intoget_conversion_rate`:

    def get_conversion_rate(self, timestamp: datetime, from_asset: str, to_asset: str, exchange: str) -> Optional[RP2Decimal]:
        result: Optional[RP2Decimal] = None
        historical_bar: Optional[HistoricalBar] = None
        key: AssetPairAndTimestamp = AssetPairAndTimestamp(timestamp, from_asset, to_asset, exchange)
        log_message_qualifier: str = ""
        if key in self.__cache:
            historical_bar = self.__cache[key]
            log_message_qualifier = "cache of "
        else:
            historical_bar = self.get_historic_bar_from_native_source(timestamp, from_asset, to_asset, exchange)
            if historical_bar:
                self.__cache[key] = historical_bar

        if historical_bar:
            result = historical_bar.derive_transaction_price(timestamp, self.__historical_price_type)
            LOGGER.debug(
                "Fetched %s conversion rate %s for %s/%s->%s from %splugin %s: %s",
                self.__historical_price_type,
                result,
                timestamp,
                from_asset,
                to_asset,
                log_message_qualifier,
                self.name(),
                historical_bar,
            )

        return result

Here, It appears self.get_historic_bar_from_native_source(timestamp, from_asset, to_asset, exchange) is returning None so diving into there:

    def get_historic_bar_from_native_source(self, timestamp: datetime, from_asset: str, to_asset: str, exchange: str) -> Optional[HistoricalBar]:
        result: Optional[HistoricalBar] = None
        time_granularity: List[int] = [60, 300, 900, 3600, 21600, 86400]
        # Coinbase API expects UTC timestamps only, see the forum discussion here:
        # https://forums.coinbasecloud.dev/t/invalid-end-on-product-candles-endpoint/320
        utc_timestamp = timestamp.astimezone(timezone.utc)
        from_timestamp: str = utc_timestamp.strftime("%Y-%m-%d-%H-%M")
        retry_count: int = 0

        while retry_count < len(time_granularity):
            try:
                seconds = time_granularity[retry_count]
                to_timestamp: str = (utc_timestamp + timedelta(seconds=seconds)).strftime("%Y-%m-%d-%H-%M")
                historical_data = HistoricalData(f"{from_asset}-{to_asset}", seconds, from_timestamp, to_timestamp, verbose=False).retrieve_data()
                historical_data.index = historical_data.index.tz_localize("UTC")  # The returned timestamps in the index are timezone naive
                historical_data_series = historical_data.reset_index().iloc[0]
                result = HistoricalBar(
                    duration=timedelta(seconds=seconds),
                    timestamp=historical_data_series.time,
                    open=RP2Decimal(str(historical_data_series.open)),
                    high=RP2Decimal(str(historical_data_series.high)),
                    low=RP2Decimal(str(historical_data_series.low)),
                    close=RP2Decimal(str(historical_data_series.close)),
                    volume=RP2Decimal(str(historical_data_series.volume)),
                )
                break
            except ValueError:
                retry_count += 1

        return result

The ValueError exception is being fired at the historical_data = HistoricalData(f"{from_asset}-{to_asset}", seconds, from_timestamp, to_timestamp, verbose=False).retrieve_data() line. Rerunning that line in the console of VS Code while debugging I see details on the error:

Traceback (most recent call last):
  File "/media/christopher/HDD/git/dali-rp2/src/dali/plugin/pair_converter/historic_crypto.py", line 45, in get_historic_bar_from_native_source
    historical_data = HistoricalData(f"{from_asset}-{to_asset}", seconds, from_timestamp, to_timestamp, verbose=False).retrieve_data()
  File "/media/christopher/HDD/git/dali-rp2/.venv/lib/python3.10/site-packages/Historic_Crypto/HistoricalData.py", line 130, in retrieve_data
    data.columns = ["time", "low", "high", "open", "close", "volume"]
  File "/media/christopher/HDD/git/dali-rp2/.venv/lib/python3.10/site-packages/pandas/core/generic.py", line 6002, in __setattr__
    return object.__setattr__(self, name, value)
  File "pandas/_libs/properties.pyx", line 69, in pandas._libs.properties.AxisProperty.__set__
  File "/media/christopher/HDD/git/dali-rp2/.venv/lib/python3.10/site-packages/pandas/core/generic.py", line 730, in _set_axis
    self._mgr.set_axis(axis, labels)
  File "/media/christopher/HDD/git/dali-rp2/.venv/lib/python3.10/site-packages/pandas/core/internals/managers.py", line 225, in set_axis
    self._validate_set_axis(axis, new_labels)
  File "/media/christopher/HDD/git/dali-rp2/.venv/lib/python3.10/site-packages/pandas/core/internals/base.py", line 70, in _validate_set_axis
    raise ValueError(
ValueError: Length mismatch: Expected axis has 0 elements, new values have 6 elements

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/media/christopher/HDD/git/dali-rp2/.venv/lib/python3.10/site-packages/Historic_Crypto/HistoricalData.py", line 130, in retrieve_data
    data.columns = ["time", "low", "high", "open", "close", "volume"]
  File "/media/christopher/HDD/git/dali-rp2/.venv/lib/python3.10/site-packages/pandas/core/generic.py", line 6002, in __setattr__
    return object.__setattr__(self, name, value)
  File "pandas/_libs/properties.pyx", line 69, in pandas._libs.properties.AxisProperty.__set__
  File "/media/christopher/HDD/git/dali-rp2/.venv/lib/python3.10/site-packages/pandas/core/generic.py", line 730, in _set_axis
    self._mgr.set_axis(axis, labels)
  File "/media/christopher/HDD/git/dali-rp2/.venv/lib/python3.10/site-packages/pandas/core/internals/managers.py", line 225, in set_axis
    self._validate_set_axis(axis, new_labels)
  File "/media/christopher/HDD/git/dali-rp2/.venv/lib/python3.10/site-packages/pandas/core/internals/base.py", line 70, in _validate_set_axis
    raise ValueError(
ValueError: Length mismatch: Expected axis has 0 elements, new values have 6 elements

And a code snippet from HistoricalData.py:

if response.status_code in [200, 201, 202, 203, 204]:
                if self.verbose:
                    print('Retrieved Data from Coinbase Pro API.')
                data = pd.DataFrame(json.loads(response.text))
                data.columns = ["time", "low", "high", "open", "close", "volume"]

Here it appears the the HTTP response from coinbase is responding with 200 (success), but the response.text is empty, resulting in an empty DataFrame, and then trying to change the column names when there are no columns, results in this Length mismatch error. Maybe this is intentional? Not sure why coinbase would response with success, but with no response.text if this was working as expected, but I'm no HTTP wiz, so maybe this is just how these things work?

Moving on, after HistoricalData.py loops through all the values of time_granularity, and fails each time, we return to transaction_resolver.py, with this time the pair_converter being ccxt.PairConverterPlugin. Stepping into its get_historic_bar_from_native_source:

    def get_historic_bar_from_native_source(self, timestamp: datetime, from_asset: str, to_asset: str, exchange: str) -> Optional[HistoricalBar]:
        self.__logger.debug("Converting %s to %s", from_asset, to_asset)

        # If both assets are fiat, skip further processing
        if self._is_fiat_pair(from_asset, to_asset):
            return self._get_fiat_exchange_rate(timestamp, from_asset, to_asset)

        if exchange == Keyword.UNKNOWN.value or exchange not in _EXCHANGE_DICT or self.__exchange_locked:
            if self.__exchange_locked:
                self.__logger.debug("Price routing locked to %s type for %s.", self.__default_exchange, exchange)
            else:
                self.__logger.debug("Using default exchange %s type for %s.", self.__default_exchange, exchange)
            exchange = self.__default_exchange

        # The exchange could have been added as an alt; if so markets wouldn't have been built
        if exchange not in self.__exchanges or exchange not in self.__exchange_markets:
            if self.__exchange_locked:
                self._add_exchange_to_memcache(self.__default_exchange)
            elif exchange in _EXCHANGE_DICT:
                self._add_exchange_to_memcache(exchange)
            else:
                self.__logger.error("WARNING: Unrecognized Exchange: %s. Please open an issue at %s", exchange, self.issues_url)
                return None

        current_markets = self.__exchange_markets[exchange]
        current_graph = self.__exchange_graphs[exchange]
        from_asset_vertex: Optional[Vertex[str]] = current_graph.get_vertex(from_asset)
        to_asset_vertex: Optional[Vertex[str]] = current_graph.get_vertex(to_asset)
        market_symbol = from_asset + to_asset
        result: Optional[HistoricalBar] = None
        pricing_path: Optional[Iterator[Vertex[str]]] = None

        # TO BE IMPLEMENTED - bypass routing if conversion can be done with one market on the exchange
        if market_symbol in current_markets and (exchange in current_markets[market_symbol]):
            self.__logger.debug("Found market - %s on single exchange, skipping routing.", market_symbol)
            result = self.find_historical_bar(from_asset, to_asset, timestamp, exchange)
            return result
        # else:
        # Graph building goes here.

        if not from_asset_vertex or not to_asset_vertex:
            raise RP2RuntimeError(f"The asset {from_asset} or {to_asset} is missing from graph")
        pricing_path = current_graph.dijkstra(from_asset_vertex, to_asset_vertex, False)

        if pricing_path is None:
            self.__logger.debug("No path found for %s to %s. Please open an issue at %s.", from_asset, to_asset, self.issues_url)
            return None

        pricing_path_list: List[str] = [v.name for v in pricing_path]
        self.__logger.debug("Found path - %s", pricing_path_list)

        conversion_route: List[AssetPairAndHistoricalPrice] = []
        last_node: Optional[str] = None
        hop_bar: Optional[HistoricalBar] = None

        # Build conversion stack, we will iterate over this to find the price for each conversion
        # Then multiply them together to get our final price.
        for node in pricing_path_list:
            if last_node:
                conversion_route.append(
                    AssetPairAndHistoricalPrice(
                        from_asset=last_node,
                        to_asset=node,
                        exchange=current_markets[(last_node + node)][0],
                        historical_data=None,
                    )
                )

            last_node = node

        for i, hop_data in enumerate(conversion_route):
            if self._is_fiat_pair(hop_data.from_asset, hop_data.to_asset):
                hop_bar = self._get_fiat_exchange_rate(timestamp, hop_data.from_asset, hop_data.to_asset)
            else:
                hop_bar = self.find_historical_bar(hop_data.from_asset, hop_data.to_asset, timestamp, hop_data.exchange)

            if hop_bar is not None:
                # Replacing an immutable attribute
                conversion_route[i] = conversion_route[i]._replace(historical_data=hop_bar)
            else:
                self.__logger.debug(
                    """No pricing data found for hop. This could be caused by airdropped
                    coins that do not have a market yet. Market - %s%s, Timestamp - %s, Exchange - %s""",
                    hop_data.from_asset,
                    hop_data.to_asset,
                    timestamp,
                    hop_data.exchange,
                )
            if result is not None:
                # TO BE IMPLEMENTED - override Historical Bar * to multiply two bars?
                result = HistoricalBar(
                    duration=max(result.duration, hop_bar.duration),  # type: ignore
                    timestamp=timestamp,
                    open=(result.open * hop_bar.open),  # type: ignore
                    high=(result.high * hop_bar.high),  # type: ignore
                    low=(result.low * hop_bar.low),  # type: ignore
                    close=(result.close * hop_bar.close),  # type: ignore
                    volume=(result.volume + hop_bar.volume),  # type: ignore
                )
            else:
                result = hop_bar

        return result

As a mental note for this: the exchange starts as Kraken, which is supposedly the default, but a the AssetPairAndHistoricalPrice.exchange value resolves to Gate on the line exchange=current_markets[(last_node + node)][0],

Now we jump into hop_bar = self.find_historical_bar(hop_data.from_asset, hop_data.to_asset, timestamp, hop_data.exchange) and eventually end up here: historical_data = current_exchange.fetchOHLCV(f"{from_asset}/{to_asset}", timeframe, ms_timestamp, 1)

I notice that the code from gate.py doesn't match the one being used in python, so likely dali is using some older version of this? Anyway, there is an exception firing at:

market = self.market(symbol)

The exception being:

BadSymbol('gateio does not have market symbol BOBA/USD')

This seems to repeat itself until the retry until the request_count exceeds 9.

macanudo527 commented 1 year ago

I think this is simply a bug caused by a typo. The market was set as BOBA/USD, but that market doesn't exist on Gate.io. However, BOBA/USDT does.

Did you try changing lines 105 and 120 to BOBA/USDT, which is a market that exists on Gate.io.

topherbuckley commented 1 year ago

@macanudo527 you got it! I overthought that quite a bit it seems. Thanks!

topherbuckley commented 1 year ago

@macanudo527 after fixing a bunch of other fatal errors I was running into, I'm back on to some of these it seems. Right now I'm failing to find the asset 'ARK' on the graph, and the transaction in question is for ARK<->USD on binance some years ago. Looking on coinmarket cap, there doesn't appear to be any market that currently supports this direct conversion any longer. Any recommendations on how to patch for this? I'm guessing this is where your dijkstra graph should come into play, so if you have any hints as to where I should start, or if there is some other way to do a quick hint-hack.

I'll come back to this tomorrow. Thanks in advance for any foresight.

Here is my backtrace btw:

ERROR: Fatal exception occurred:
Traceback (most recent call last):
  File "/media/christopher/HDD/git/dali-rp2/src/dali/dali_main.py", line 181, in _dali_main_internal
    resolved_transactions: List[AbstractTransaction] = resolve_transactions(transactions, dali_configuration, args.read_spot_price_from_web)
  File "/media/christopher/HDD/git/dali-rp2/src/dali/transaction_resolver.py", line 246, in resolve_transactions
    transaction = _update_spot_price_from_web(transaction, global_configuration)
  File "/media/christopher/HDD/git/dali-rp2/src/dali/transaction_resolver.py", line 136, in _update_spot_price_from_web
    conversion: RateAndPairConverter = _get_pair_conversion_rate(
  File "/media/christopher/HDD/git/dali-rp2/src/dali/transaction_resolver.py", line 108, in _get_pair_conversion_rate
    rate = cast(AbstractPairConverterPlugin, pair_converter).get_conversion_rate(timestamp, from_asset, to_asset, exchange)
  File "/media/christopher/HDD/git/dali-rp2/src/dali/abstract_pair_converter_plugin.py", line 182, in get_conversion_rate
    historical_bar = self.get_historic_bar_from_native_source(timestamp, from_asset, to_asset, exchange)
  File "/media/christopher/HDD/git/dali-rp2/src/dali/plugin/pair_converter/ccxt.py", line 258, in get_historic_bar_from_native_source
    raise RP2RuntimeError(f"The asset {from_asset} or {to_asset} is missing from graph")
rp2.rp2_error.RP2RuntimeError: The asset ARK or USD is missing from graph
macanudo527 commented 1 year ago

You just need to add the "entry" market and the Djikstra search will route to your target asset. For example, there is a Binance.com market ARK/BUSD you can add if you have access to Binance.com. I don't remember if you are based in the States or not. Binance.com pricing is unavailable to the States.

You could try adding Upbit if you want to go that route. They have a ARK/KRW market. It seems like ARK is super popular in korea. Adding an exchange is fairly easy. You just need to import the appropriate class from CCXT and declare the stubs in https://github.com/eprbell/dali-rp2/blob/main/src/stubs/ccxt.pyi

Personally, I would try Binance, but if that isn't available Upbit. However, it is a little tricky since I'm guessing you don't actually have access to that exchange and couldn't trade there. It might be hard to justify in an audit.

topherbuckley commented 1 year ago

@macanudo527 thanks for the info! I'll try it out.

I don't remember if you are based in the States or not. Binance.com pricing is unavailable to the States. ... It might be hard to justify in an audit.

Very interesting tangents you brought up there.

I'm currently in the US visiting family for a few months, but a resident of Japan. Last I checked, a year or so ago binance.com had disabled my account for being a US citizen regardless of residency. I had signed up for an account sometime in 2016 if I'm remembering correctly, and it remained largely unused but it seemed this was ok with regulations back then as I never falsified any information about my identity/residency/citizenship. So in summary, I need pricing for an exchange that was ok for me to use at one point, but due to regulatory changes, I am no longer able to access. Maybe this is a question for another forum, (or tax expert), but I'd guess its ok for me to look up the price on binance as I was apparently trading on it when this was still allowed. I don't have any trades on there after the regulations changed.

I would assume others are in a similar situation (i.e. having trades on an exchange like Binance and then were booted due to regulatory changes). I agree that it would look suspicious if you are getting the price from an exchange you are not allowed to access, though one might argue that the price can't be SO different across exchanges. That being said, maybe some sort of input to the configuration file stating date ranges to use certain exchanges where ccxt would use one route during one time period and another during another? Sounds complicated, and I wouldn't have any idea how to approach this, but do you think it is worth opening a discussion on this?

macanudo527 commented 1 year ago

Binance.com's public REST API is unavailable in the states, but it's available in Japan, and most other places whether or not you have an account with them.

You just need access to the public API for ohlcv candles (ie. Pricing data).

macanudo527 commented 1 year ago

This seems resolved via #171