currencyrate: propagate http errors to currencyrate rpc if a source is provided

[daywalker90]

specifically coindesk was constantly hitting API rate limits causing our tests to fail
so lets unit tests all endpoints with a snapshot of real responses and only allow for http
error 429 in integration tests

Also fix a flake in test_bkpr_currencyrate_persisted that would pick up a cached rate from CLN's own caching

[nGoline]

I believe this level of mocking would fit a Unit Test better than an Integration Test. This error helps us tell, like you said:

If the actual API's change, break, or vanish...

Wouldn't it be better to catch and propagate the error back to the test? This way we would know it's a HTTP 429 Too Many Requests and safely disregard that one provider, whilst still testing the integration between all the moving parts for this test.

[daywalker90]

I've reworked the PR to propagate http errors and replaced the mocked tests with unit tests. The integration tests now only error on errors that are not http errors.

[daywalker90]

test_bkpr_currencyrate_persisted was flaky locally. CLN's caches the rate by looking up the last rate and allow for a 60s old rates to be used without lookup. What i don't understand is why the test would sometimes pass.

[daywalker90]

Wouldn't it be better to catch and propagate the error back to the test? This way we would know it's a HTTP 429 Too Many Requests and safely disregard that one provider, whilst still testing the integration between all the moving parts for this test.

So cln-currencyrate always uses all sources since almost all commands are interested in them except for currencyrate and only if you provide an optional source argument which was added later on. I can't error all the commands just because one source failed, no one wants that. So to detect failures i opted for a higher log level that would make the tests fail, but currencyrate is used whenever bkpr-currency is enabled, which it is in a bunch of other tests in test_bookkeeper.py. So i'm restructuring cln-currencyrate a bit to let currencyrate report that error.

[nGoline]

That looks good, just a few comments...

[daywalker90]

Ugh now coindesk responded with HTTP error 401 Unauthorized API key required instead of 429. I added 401 to allowed errors as well. It's not that they actually require a API key, it works with my IP without it, but it is just their new way to say: API limit reached...

[nGoline]

LGTM