multi: custom fiat prices #122

ellemouton · 2021-05-21T10:05:48Z

This PR allows a user to specify their own fiat price points in a CSV file in the following format:

Unix Timestamp [seconds]	Price of 1 BTC in chosen currency
1621246440	10.1
1621249080	123.45
...	...

Then you can do the following:

$ frcli audit --enable_fiat --fiat_backend=custom --prices_csv_path=/path/to/pricedata.csv --custom_price_currency=ZAR --start_time=1621246480 --end_time=1621249110

and

$ frcli fiat --amt_msat=100000000000  --fiat_backend=custom --prices_csv_path=/path/to/pricedata.csv --custom_price_currency=ZAR

Fixes #113

fiat/customprices.go

cmd/frcli/csv.go

carlaKC

Looking great!

Mostly comments re how we validate this new input, could use slightly stricter checks so it's very clear what we're requiring.
Sorry it took me so long! Also could move some of the renames into their own commits, but that is a hassle so non-blocking.

accounting/conversions.go

fiat/coincap_api_test.go

fiat/fiat.go

frdrpc/faraday.proto

cmd/frcli/csv.go

fiat/customprices.go

fiat/prices.go

cmd/frcli/csv.go

frdrpc/node_audit.go

ellemouton · 2021-06-09T13:07:39Z

Updated and split out some commits. I still gotta add the price point time stamp filtering on the frcli side. Will do that soon 👍

ellemouton · 2021-06-10T08:56:32Z

cool, the last commit does the timestamp filtering in frcli

cmd/frcli/fiat_estimate.go

carlaKC

A few last nits from me, looking spectacular.

I like having the frcli side filtering, saves us from shuffling a ton of unnecessary data back and forth.

The last thing that I think we need is to add some rpcserver side validation of the custom price data. The way that our price logic works is that we always need a timestamp that is before our start date to be able to get prices. This should live in the server, so that cli and rpc clients are validated. Since prices can be provided at varying granularity (eg, yearly, monthly, daily), it's difficult to know what other checks we should enforce on that data, but at least <= start date should be required.

carlaKC · 2021-06-17T08:07:08Z

fiat/coincap_api.go

 	// coinCapHistoryAPI is the endpoint we hit for historical price data.
 	coinCapHistoryAPI = "https://api.coincap.io/v2/assets/bitcoin/history"
+
+	// coinCapDefaultCurrency is the currency that the price data returned by the


is this wrapped at 80? Looks a leetle off to me.

carlaKC · 2021-06-17T08:10:21Z

cmd/frcli/node_audit.go

 	}()

+	if len(report.Reports) > 0 {
+		CSVHeaders = strings.Replace(


Instead of strings.Replace we could have:

var CSVHeaders = "Timestamp,OnChain,Type,Category,Amount(Msat),Amount(%v),TxID,Reference,BTCPrice,BTCTimestamp,Note" headers = fmt.Sprintf(CSVHeaders, report.Reports[0].BtcPrice.Currency)

Just a little shorter, and gives us options to add more variable fields going forward.

accounting/config.go

carlaKC · 2021-06-17T08:16:08Z

fiat/prices.go

+
+// validatePriceSourceConfig checks that the PriceSourceConfig fields are valid
+// given the chosen price backend.
+func validatePriceSourceConfig(cfg *PriceSourceConfig) error {


could also be func (p *PriceSourceConfig) validate()error and then we only validate if non-nil.

frdrpc/node_audit.go

cmd/frcli/fiat_estimate.go

carlaKC · 2021-08-27T06:42:24Z

Would be great if we can pick this up again!

Rename the USDPrice struct to Price so that it can be used to represent more than 1 currency.

Rename usdPrice func type to fiatPrice so that the name is appropriate for multiple fiat currencies.

Rename the MsatToUSD functino to MsatToFiat so that the name is appropriate for multiple currencies.

Add Currency field to the price struct so that Price can be used to represent any currency.

Update the RPC methods (exchange rate and node report) so that they show the fiat currency that the bitcoin price is quoted in.

This commit adds a new implementation of the fiatBackend interface which uses user provided prices data.

Add a PriceSourceConfig struct to consolidate the options required for initialising a new PriceSource.

Add new CustomPriceBackend option to NewPriceSource function that initialises the customPrices implementation of the fiatBackend interface.

Rename the csv function to writeToCSV so that the csv package can be imported in a future commit without needing to rename the import.

This commit adds the options required for the custom fiat price data to the RPC methods. These methods include the NodeAudit and ExchangeRate methods.

This commit filters the custom price points provided using the given start and end timestamps. This is done so that only the necessary time stamps are sent to the server.

This commit adds a server side check that ensures that at least one price point is provided that has a timestamp preceding the start time.

ellemouton · 2021-08-27T15:12:23Z

Updated @carlaKC :)

The last thing that I think we need is to add some rpcserver side validation of the custom price data.

Added this in the last commit

carlaKC

Awesome to wrap this up!

Just one Q left, probably because I've forgotten context here :')

carlaKC · 2021-08-30T07:19:10Z

fiat/prices.go

+// given the chosen price backend.
+func (cfg *PriceSourceConfig) validatePriceSourceConfig() error {
+	switch cfg.Backend {
+	case UnknownPriceBackend, CoinCapPriceBackend:


Why do we require granularity set for UnknownPriceBackend? Wouldn't we only expect it for coincap?

ellemouton commented May 21, 2021

View reviewed changes

fiat/customprices.go Show resolved Hide resolved

ellemouton commented May 21, 2021

View reviewed changes

cmd/frcli/csv.go Show resolved Hide resolved

ellemouton force-pushed the customPrices branch from 733150f to f234dcd Compare May 24, 2021 08:47

carlaKC self-requested a review May 25, 2021 08:12

carlaKC reviewed Jun 7, 2021

View reviewed changes

ellemouton force-pushed the customPrices branch from f234dcd to 4936677 Compare June 9, 2021 12:54

ellemouton force-pushed the customPrices branch from 4936677 to 95e66f8 Compare June 10, 2021 08:34

ellemouton commented Jun 10, 2021

View reviewed changes

cmd/frcli/fiat_estimate.go Show resolved Hide resolved

carlaKC mentioned this pull request Jun 17, 2021

fiat: Expand currency offering #38

Closed

carlaKC reviewed Jun 17, 2021

View reviewed changes

ellemouton added 12 commits August 27, 2021 15:54

multi: rename USDPrice struct to Price

8b213c7

Rename the USDPrice struct to Price so that it can be used to represent more than 1 currency.

accounting: rename usdPrice func type to fiatPrice

b6404b7

Rename usdPrice func type to fiatPrice so that the name is appropriate for multiple fiat currencies.

multi: rename MsatToUSD func to MsatToFiat

4a7199f

Rename the MsatToUSD functino to MsatToFiat so that the name is appropriate for multiple currencies.

fiat: add Currency field to the Price struct

dffb009

Add Currency field to the price struct so that Price can be used to represent any currency.

multi: update RPC to show the fiat currency

36188eb

Update the RPC methods (exchange rate and node report) so that they show the fiat currency that the bitcoin price is quoted in.

fiat: add customprice impl of fiatBackend

66ad73a

This commit adds a new implementation of the fiatBackend interface which uses user provided prices data.

multi: add PriceSourceConfig struct

71b47dc

Add a PriceSourceConfig struct to consolidate the options required for initialising a new PriceSource.

fiat: add new custom price case to NewPriceSource

5047621

Add new CustomPriceBackend option to NewPriceSource function that initialises the customPrices implementation of the fiatBackend interface.

cmd/frcli: rename csv function to writeToCSV

bffe524

Rename the csv function to writeToCSV so that the csv package can be imported in a future commit without needing to rename the import.

multi: add custom price options to RPC methods

f064bb2

This commit adds the options required for the custom fiat price data to the RPC methods. These methods include the NodeAudit and ExchangeRate methods.

frcli: filter custom prices by timestamp

e478853

This commit filters the custom price points provided using the given start and end timestamps. This is done so that only the necessary time stamps are sent to the server.

frdrpc: validate custom pricepoints

07ec73e

This commit adds a server side check that ensures that at least one price point is provided that has a timestamp preceding the start time.

ellemouton force-pushed the customPrices branch from 95e66f8 to 07ec73e Compare August 27, 2021 15:09

carlaKC approved these changes Aug 30, 2021

View reviewed changes

carlaKC merged commit f953eea into lightninglabs:master Aug 30, 2021

ellemouton deleted the customPrices branch August 30, 2021 07:53

multi: custom fiat prices #122

multi: custom fiat prices #122

Uh oh!

Conversation

ellemouton commented May 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

carlaKC left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ellemouton commented Jun 9, 2021

Uh oh!

ellemouton commented Jun 10, 2021

Uh oh!

Uh oh!

carlaKC left a comment

Choose a reason for hiding this comment

Uh oh!

carlaKC Jun 17, 2021

Choose a reason for hiding this comment

Uh oh!

carlaKC Jun 17, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

carlaKC Jun 17, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

carlaKC commented Aug 27, 2021

Uh oh!

ellemouton commented Aug 27, 2021

Uh oh!

carlaKC left a comment

Choose a reason for hiding this comment

Uh oh!

carlaKC Aug 30, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ellemouton commented May 21, 2021 •

edited

Loading