fiat: add historical price estimation using coincap api #26
Conversation
fiat/fiat.go
Outdated
| maxRetries = 3 | ||
|
|
||
| // retrySleep is the period we backoff for between tries. | ||
| retrySleep = time.Millisecond * 50 |
There was a problem hiding this comment.
This is probably a bit aggressive, resulting in a lot of requests quickly. I'd probably wait for at least half a second.
| ) | ||
|
|
||
| // Subsystem defines the logging code for this subsystem. | ||
| const Subsystem = "FIAT" |
fiat/coincap_api.go
Outdated
| // The number of requests we require may be a fraction, so we use a | ||
| // float to ensure that we perform an accurate number of request. | ||
| var i float64 | ||
| for ; i < requiredRequests; i++ { |
There was a problem hiding this comment.
nit: can be for i := float64(0); i < ...
fiat/coincap_api.go
Outdated
| return c.query(queryStart, queryEnd, c.granularity) | ||
| } | ||
|
|
||
| // Query the api for this page of data. We allow retires at this |
| int64 timestamp = 3; | ||
| } | ||
|
|
||
| repeated PriceRequest requests = 1; |
There was a problem hiding this comment.
Is there a specific reason to allow multiple requests to be sent at a time? The API is not really intuitive that way with the request ID and the map as a response. But maybe it's needed for a follow-up PR?
There was a problem hiding this comment.
This functionality is mostly needed to produce fiat quotes en-masse for an accounting spreadsheet. So it'll be much more efficient to make one large query.
However, that's going to be done internally in faraday, so rpc could be simplified to just be a single request like the cli? Really just added it for easy testing/ because we might as well have the feature.
There was a problem hiding this comment.
It's okay leaving it that way, was mostly curious. Could be useful for other internal projects to batch multiple requests together.
Add a retry query method which can be used to repetitively query an external api, with sleeps in between failed queries. We provide a quit channel which can be used to terminate this process early if required.
Add a new package which allows us to obtain historical bitcoin prices. The first external source we query is coincap. Their api allows specification of granularity of data - the time period over which prices are aggregated. The api has a limit on the period of time you may query per granularity - effectively, a limit on the number of datapoints they will allow. To allow lower levels of granularity, we allow breaking up of our call into a maximum of 5 separate api calls. This granularity setting also affects the data that is returned when you are under the limit. If request one day's granularity, then only query for half a day's data, then it will not return any data points. To account for this, we add a minimum period check as well. To account for api failures, we allow 3 retries per api call, with a sleep inbetween. To prevent blocking, we pass context in. Note that this PR does not have the actual API call and parsing logic in it. These will be added in followup commits.
carlaKC
force-pushed
the
fiat-historicalprices
branch
from
47fcf5a
to
3b98219
Compare
To create accounting reports, we require fiat estimates for BTC prices. This PR adds a fiat package which gets historical prices from coincap's api, and produces price quotes for a set of timestamped BTC values.
This package is internally required for spreadsheet generation, as in #23, but may be a useful feature for end users as well. For this reason, rpc/cli endpoints are added to expose this functionality (and allow easy testing).
The coincap api was chosen because it is currently used in our internal spreadsheet generation. The design of this package is intended to allow addition of other data source, should this become a priority.
One open US question, is how we deal with msat. Currently this PR rounds to the nearest satoshi, since BTC cannot (yet 🤔 ⚡ ) be exchanged for fiat in this unit. However, sub-satoshi amounts are common in ln, so a rounding override which does not round down may be desirable.