Skip to content

Conversation

@daywalker90
Copy link
Collaborator

@daywalker90 daywalker90 commented Jan 14, 2026

Fixes #8825

I described my approach for doing API calls in the issue but i repeat it here:
I added 7 default price sources of which one (bitstamp) never seems to work (HTTP 403) when using a tor proxy. Out of these 6-7 CONVERT_SOURCES_COUNT (3) are chosen at random when currencyconvert is called, unless we are inside the cache time window.

I like this approach because:

  • always fresh data, maximum staleness is CACHE_DURATIONS_SECS (10)
  • median of CONVERT_SOURCES_COUNT (3) sources aslong as enough are configured and return a valid result
  • not hitting the API's rapidly within the cache time window CACHE_DURATIONS_SECS (10)
  • not hitting all the API's every time, assuming you have more than CONVERT_SOURCES_COUNT (3)

What i don't like about it:

  • outside of the cache time window the worst case response time is SOURCE_TIMEOUT_SECS (5) * n (where n is >1 if any of the initial 3 fail but is at most the number of sources configured - 2) and i expect this to be called right before doing a payment, delaying it by up to that much. If it's only used for recurring payments i would assume response time is not that critical, so i found this acceptable

To always instantly have a response, we would either have to look for API endpoints that return all currency pairs and cache them or ask the user what currency they are interested in or use historic user queries to know wich currency to track.

I also tried my best to be compatible with the python version of currencyrate, meaning i left the options and rpc methods the same, except for the msat suffix. But i can see if you want to break this since the option names inside CLN are a little under-descriptive (add-source, disable-source). I also added support for positional members when using add-source (kraken needs it).

Please let me know:

  • are you happy with the API call approach or do you prefer a different one, if so please let me know what you value more response time, data age, API rate limit etc.

  • where and how you want this documented

  • should i mark the tests as flaky since they make real-live API calls and they can easily fail if one of them doesn't return a valid result

  • The changelog has been updated in the relevant commit(s) according to the guidelines.

  • Tests have been added or modified to reflect the changes.

  • Documentation has been reviewed and updated as needed.

  • Related issues have been listed and linked, including any that this PR closes.

  • Important All PRs must consider how to reverse any persistent changes for tools/lightning-downgrade

@Lagrang3 Lagrang3 self-requested a review January 14, 2026 16:20
@daywalker90
Copy link
Collaborator Author

Fixed the check-manpage error and disabled the plugin in test_pay.py, as it assumed we didn't have such a plugin.

@daywalker90 daywalker90 force-pushed the currencyrate-rs branch 3 times, most recently from 762d41a to d68a222 Compare January 15, 2026 00:43
Changelog-Added: new plugin currencyrate to provide `currrencyconvert` API
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

currencyconvert needs to be a built-in plugin

1 participant