Adds test for blockheight for all endpoints

[yashasvi-ranawat]

A method is added to NetworkAPI that sanitizes the endpoints; where the endpoints that are unreachable or are un-synced (based on their given blockheight) are removed.

This fixes issues like #140

[yashasvi-ranawat]

Why didn't the tests trigger?

[merc1er]

Why didn't the tests trigger?

Good question. We've always had this issue I believe. The tests are supposed to run on your fork. I'll see if there's some settings preventing this in this repo.

[merc1er]

Settings look OK...

Anyway, I'll review and test locally for now.

[ssmycelium]

This is very clever! Nice work!

[merc1er]

@yashasvi-ranawat thank you for this PR 🙏🏻

I have been testing it, and it's working great so far. But it is now taking much longer to perform any network action.

For example, get_balance():

In [7]: import time
   ...: 
   ...: start_time = time.time()
   ...: key.get_balance()
   ...: end_time = time.time()
   ...: elapsed_time = end_time - start_time
   ...: print("Time to get balance: " + str(elapsed_time) + " seconds")

# BEFORE
Time to get balance: 0.37384819984436035 seconds

# AFTER
Time to get balance: 2.398766279220581 seconds

The difference is too important. I'm thinking a way to mitigate this would be to cache the provider so the block height is not fetched every time.

[yashasvi-ranawat]

I'm thinking a way to mitigate this would be to cache the provider so the block height is not fetched every time.

I implemented a time-to-live cache for blockheight with cache time limit of 120s.

Implementation	Wall time
No blockheight based sanitation	365ms
Cached blockheight, first hit	2.71s
Cached blockheight, second hit	1.05s
Cached blockheight, no unresponsive bitcoin.com api, first hit	1.34s
Cached blockheight, no unresponsive bitcoin.com api, second hit	514ms

We can remove the second default API in BitcoinDotComAPI, which is the, now unresponsive, rest.bitcoin.com. It causes much delay in blockheight retrieval.

[yashasvi-ranawat]

oh python 3.8 functools does not have cache! Although it'll be at its end-of-life this October; don't know if I want to implement a custom cache for it.

[merc1er]

Thanks! Yes we can definitely remove rest.Bitcoin.com and drop Python 3.8 support.

However, there is already a caching system present in BitCash for currency conversion support: set_rate_cache_time

Why not use something like this instead? Just a question, no need to re-implement everything. I can submit a PR removing rest.Bitcoin.com in the meantime.

[merc1er]

@yashasvi-ranawat I have prepared a PR removing rest.Bitcoin.com. Feel free to review it if you have the time (I completely understand if not; in which case I will merge).

#143

[yashasvi-ranawat]

However, there is already a caching system present in BitCash for currency conversion support: set_rate_cache_time

yeah, good idea! I'll have a look.

[yashasvi-ranawat]

I had a look at the cache setup for rates API! It was similar to the one I added to utils, but was specific to use for rates API. I refactored the one in utils to cache multiple entries with LRU cache. This is now usable by both blockheight caching and rates caching. I updated the tests accordingly.

[yashasvi-ranawat]

I further refactored to caching of sanitized endpoints, instead of endpoint's blockheight. This drastically improves wall time.

Implementation	Wall time
No blockheight based sanitisation	365 ms
Cached sanitized endpoints, first hit	1.28 s
Cached sanitized endpoints, second hit	198 ms

This PR is ready for review!

[merc1er]

Outstanding work, thank you for yet another contribution!

Everything is working for me, it's much faster after the first call, and the code looks good, so I am approving.

Note that https://demo.chaingraph.cash/v1/graphql has been out of sync for a very long time, so I may consider removing it at some point.

---

Blockheight for <bitcash.network.APIs.ChaingraphAPI.ChaingraphAPI object at 0x10561f020>: 844258
Blockheight for <bitcash.network.APIs.ChaingraphAPI.ChaingraphAPI object at 0x10561f260>: 844477
Blockheight for <bitcash.network.APIs.BitcoinDotComAPI.BitcoinDotComAPI object at 0x105708bc0>: 844477
(<bitcash.network.APIs.ChaingraphAPI.ChaingraphAPI object at 0x10561f260>, <bitcash.network.APIs.BitcoinDotComAPI.BitcoinDotComAPI object at 0x105708bc0>)