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
65 stars 42 forks source link

Set unique cache keys for pair converters #149

Closed qwhelan closed 1 year ago

qwhelan commented 1 year ago

Assume a user has a config using multiple ccxt pair converter plugins - this can be done to express an explicit fallback order:

[dali.plugin.pair_converter.ccxt_binance]
...

[dali.plugin.pair_converter.ccxt_kraken]
...

The expected behavior in this case is:

The issue with the above is that both plugins have the cache_key() of CCXT-converter and the following happens instead:

Due to rate limiting at the source and the cache priority inversion, the above issue can significantly slow down dali on repeated runs and lead to almost no caching speedup.

A subtler issue can actually occur when setting fiat_priority if it is modified between dali runs: as the fiat exchange rate is applied before adding the result to the cache, any changes to fiat_priority will not be reflected.

The solution is to introduce a unique cache key that incorporates settings that may invalidate the cache and to require that users not have two identical pair converter plugins.

With this change, the ccxt_binance plugin would write to .dali_cache/CCXT-converter_Binance.com and ccxt_kraken plugin would write to .dali_cache/CCXT-converter_Kraken if fiat_priority is not set.

I'm indifferent on the format of the cache_key and am open to suggestions on better ones.

macanudo527 commented 1 year ago

Sorry, I don't see any problems with this. We can merge it.