taproot-assets icon indicating copy to clipboard operation
taproot-assets copied to clipboard

[feature]: add a function, `rfqmath.FixedPointFromUint64_Decimal_Display`

Open ZZiigguurraatt opened this issue 10 months ago • 8 comments

There is a lot of confusion when running a price oracle server on how to properly input the exchange rate. One example of this is here: https://lightningcommunity.slack.com/archives/C03B3556HQ8/p1738866875640909?thread_ts=1738748330.746709&cid=C03B3556HQ8 .

The basic-price-oracle example uses the rfqmath.FixedPointFromUint64 function, and that is where a lot of the confusions arises.

https://github.com/lightninglabs/taproot-assets/blob/28c4ea8e019bb5a52e59401f66b0a5eebf0a0eee/docs/examples/basic-price-oracle/main.go#L111-L126

https://github.com/lightninglabs/taproot-assets/blob/28c4ea8e019bb5a52e59401f66b0a5eebf0a0eee/rfqmath/fixed_point.go#L189-L199

What is very confusing to new users is the "scale". It is recommended that this be the same as the decimal display that was defined when creating the asset. This leads users to think that the "coefficient" be defined in terms of decimal display asset units and then the exchange rate can be off by an order of magnitude of the decimal display. The "scale" parameter is recommended to be the same as the decimal display in order to reduce arithmetic round off error, but it doesn't really change the exchange rate itself.

I'd recommend a new function be created, rfqmath.FixedPointFromUint64_Decimal_Display, where the the exchange rate in terms of decimal display assets can be input, and then the decimal display digits as the second input. Then, convert the exchange rate from decimal display asset units to base asset units and use that as the coefficient input to rfqmath.FixedPointFromUint64. Then, also use the decimal display digits as the scale input to rfqmath.FixedPointFromUint64.

See also, the definition of FixedPoint: https://github.com/lightninglabs/taproot-assets/blob/3a0556c25d5c196cd7abd690d4cbb1c91369a4f9/taprpc/priceoraclerpc/price_oracle.proto#L26-L57 .

ZZiigguurraatt avatar Feb 13 '25 16:02 ZZiigguurraatt

Thanks for looking into this. What would have helped me most would be better examples in the price-oracle-example code. So I suggest to keep the code as is but improve the comments. For example:

 	// As an example, the purchase rate is $42,000 per BTC.
        // The returned value from this function must represent the
        // number of base asset units per BTC. With a decimal display
        // of 6, we should return the amount 42,000,000,000.
        // But we need to return the number as a FixedPoint, which
        // contains a coefficient (42,000,000,000) and a scale which is
        // used to increase precision in arithmetics. The scale can be
        // anything, but it's recommended to use the same value as
        // decimal_display. In our example, a reasonable value for
        // scale is thus 6. But it's going to work with 5 or 8 too.

kallerosenbaum avatar Feb 14 '25 15:02 kallerosenbaum

I think your coefficient is going to be 42,000,000,000,000,000 with a scale of 6 and a decimal display of 6.

With a decimal display of 6 and $42,000 per BTC, 42,000,000,000 is going to be the "fractional value" defined here: https://github.com/lightninglabs/taproot-assets/blob/3a0556c25d5c196cd7abd690d4cbb1c91369a4f9/taprpc/priceoraclerpc/price_oracle.proto#L26-L57

ZZiigguurraatt avatar Feb 20 '25 08:02 ZZiigguurraatt

Gaahh, yes you're right of couse. Thanks for pointing it out. So my suggested comment should be:

 	// As an example, the purchase rate is $42,000 per BTC.
        // The returned value from this function must represent the
        // number of base asset units per BTC. With a decimal display
        // of 6, we should return the amount 42,000,000,000.
        // But we need to return the number as a FixedPoint, which
        // contains a coefficient (42,000,000,000,000,000) and a scale which is
        // used to increase precision in arithmetics. The scale can be
        // anything, but it's recommended to use the same value as
        // decimal_display. In our example, a reasonable value for
        // scale is thus 6. But it's going to work with 5 or 8 too.

kallerosenbaum avatar Feb 20 '25 10:02 kallerosenbaum

yes, very confusing, that's why I think we need more than just better documentation to clear some of this stuff up for people!

ZZiigguurraatt avatar Feb 20 '25 23:02 ZZiigguurraatt

The FixedPointFromUint64 function will do the scaling for you. You provide the base value, and it does the multiplication needed to scale it up.

The base value is the amount of assets to 1 BTC. If you didn't have any decimal display at all, then it would be 100k. However if you have a decimal display of 3, then this means 1000 units is actually $1, so it would be 100 million.

Roasbeef avatar Feb 21 '25 01:02 Roasbeef

Leo and I ran a number of tests on this using a Signet testing asset, Stablesigs. This asset was minted as a mock stablecoin, and was minted with a decimal display of 6.

Asset:				        Stablesigs
Decimal display: 		        6
BTC price: 			        100k
Assets equal to 1 USD:	1,000,000

We tested two different setups both using my Signet2 node as the Edgenode. The first was using the below testing config.

taproot-assets.experimental.rfq.priceoracleaddress=use_mock_price_oracle_service_promise_to_not_use_on_mainnet
taproot-assets.experimental.rfq.mockoracleassetsperbtc=100000000000

And the second using the demo price oracle with the below numbers on lines 117 & 125.

100_000, 6,

It’s easy to confuse decimal display and scale. The 6 in the above setting is scale and not decimal display

For these tests Leo generated a 10,000 sats invoice that he then paid in Stablesigs facilitated by my edge node.

Leo Signet node → (Stablesigs) → Hannah Signet Node2 Edgenode → (Sats) → broader Signet LN → (Sats) -> Leo’s Signet blink wallet

In the first test using the config only, Leo paid the 10k sats invoice($10 USD value) using 10,002,222 Satblesigs which is essentially $10 USD value + a %0.0002 fee. Remember 1,000,000 Stablesigs are equal to 1 USD at the $100k price.

However, when the same test is ran using the demo price oracle the 10k sats invoice was paid using only 10 Stablesigs. Off by a magnitude of 6.

We ran this test for a third time, this time manually adding the decimal display adjustment to the price oracle. This time we added the below numbers on lines 117 & 125. 100_000_000_000, 6,

After this adjustment another 10k sats payment was paid this time using the correct amount of Stablesigs + fees, 10,002,222.

So it seems that the demo price oracle can perhaps be used well in it’s current state, but the instructions would need to be updated.

HannahMR avatar Mar 26 '25 18:03 HannahMR

Introducing some consistent terminology like asset_base and asset_decimal_display across all documentation and examples could help with the confusion too. The units used in the basic-price-oracle are in terms of asset_base and NOT asset_decimal_display.

Related: For USD, we need to also make it clear in all examples and documentation if the asset_decimal_display defined when minting is for USD or USDcent. I think a recommendation of USD (not USDcent) to have a decimal display of 6 is common in most documentation and is what is used mostly in examples, but we could also clarify in the documentation and examples that that would result in an effective USDcent decimal display of 4.

ZZiigguurraatt avatar Mar 27 '25 16:03 ZZiigguurraatt

I'd recommend a new function be created, rfqmath.FixedPointFromUint64_Decimal_Display, where the the exchange rate in terms of decimal display assets can be input

We'd probably want to make this input be a float though and not an integer, so maybe the function name needs to be changed.

ZZiigguurraatt avatar Apr 09 '25 15:04 ZZiigguurraatt

I've attempted to improve the documentation here: https://github.com/lightninglabs/taproot-assets/pull/1479 I think what I've written is correct, it's been a little while since I've thought of fixed points and asset rates. Please share your feedback on the PR.

ffranr avatar Apr 17 '25 23:04 ffranr

What is very confusing to new users is the "scale". It is recommended that this be the same as the decimal display that was defined when creating the asset.

I actually couldn't find this recommendation anywhere -- is it still extant?

IMO the docs can still be further refined, but I do think that the updates that were made to the basic price oracle have better addressed any underlying confusion here than would the function proposed in the OP (or one like it). Is there any context as to why this was reopened? (cc @levmi)

jtobin avatar Sep 13 '25 05:09 jtobin

Is there any context as to why this was reopened? (cc @levmi)

The closure commit did not actually do what I asked for. It basically made a docs change but I actually asked for a new function. Also, we had gotten more feedback during a community call that people were still confused on this.

ZZiigguurraatt avatar Sep 15 '25 15:09 ZZiigguurraatt

I actually couldn't find this recommendation anywhere -- is it still extant?

I am not sure. There were a number of conversations that were had offline (with @HannahMR , @Roasbeef , and several others) about this subject and I tried to summarize many of those unclear recommendations out into the public so that they actually would be documented somewhere.

ZZiigguurraatt avatar Sep 15 '25 15:09 ZZiigguurraatt

Seems low prio. Unclear if anyone building actually needs something like this or not.

If you want to just add the function in a PR to the package, you're free to do so.

Roasbeef avatar Sep 18 '25 00:09 Roasbeef

My understanding is that the rfq confusion was resolved with @ffranr's updates in May. I have not seen any further issues since that update.

HannahMR avatar Sep 19 '25 16:09 HannahMR