TBD54566975 / tbdex

56 stars 26 forks source link

Currency representation #199

Closed diehuxx closed 9 months ago

diehuxx commented 11 months ago

Three decisions to make for how to represent currency amounts:

  1. String or number amounts:
    • Arguments in favor of String: We cannot reliably use integers because of currencies like millisatoshis. Even if we represent currencies as minor unit amounts, millisatoshis will force us to have fractional subunit amounts, which defeats the purpose. Floats risk loss of precision as they are decoded and handled in various languages.
  2. Major or minor units
    • Arguments in favor of major units: Millisatoshis render the benefits of minor units moot.
  3. Decimal Separator: . or sometimes ,
    • Some currencies/locales use a comma as a decimal separator. Should currency strings use a comma as decimal separator for those currencies?
    • Arguments for period as separator: JSON already standardizes periods as decimal separators. For consistency's sake, we should do the same in our string amounts.
andresuribe87 commented 11 months ago

I think this all makes sense. We shouldn't reinvent the wheel. Representing as decimal numbers makes sense to me.

Tangentially, I noticed that CurrencyDetails has minSubunits. What does that mean? Is it the same as precision? Should that be clarified?

mistermoe commented 11 months ago

ah thanks for creating this @diehuxx !

for a bit more context, as we were working on girlmath the following came to mind

the amount fields in tbdex messages are all subunits that are represented as string values. This feels a bit strange because the initial motivation of using subunits was because floating point arithmetic loses precision. we then also chose to represent those values as strings which was also motivated by floating point arithmetic causing precision loss. This led us to think we should do one or the other of the following:

IMO, representing amounts as major values represented as strings makes the most sense. Any amount related math should be done using the respective language's most reliable decimal libraries. representing everything as subunits doesn't help with respect to precision loss because we'll inevitably encounter decimals while doing currency conversion.

Here are all the fields we'll want to rename if we go with major units:

[!NOTE] New column has left out the full path for brevity. Assume they're there

Current New
offering.payinCurrency.minSubunits min or minAmount
offering.payinCurrency.maxSubunits max or maxAmount
offering.payoutCurrency.minSubunits min or minAmount
offering.payoutCurrency.maxSubunits max or maxAmount
offering.paymentMethods[*].feeSubunits fee
rfq.payinSubunits amount or payinAmount
quote.payin.amountSubunits amount
quote.payin.feeSubunits fee
quote.payout.amountSubunits amount
quote.payout.feeSubunits fee

aand on that note, one last mostly orthogonal thing to surface is whether we want to rename offering. payoutUnitsPerPayinUnit to offering.rate. though we can probably address this in another issue

jiyoonie9 commented 11 months ago

wanted to throw this in:

is this condition always true? rfq.payinUnits == quote.payin.amountUnits (whatever unit we end up going with) and is this a validation rule we should build into the http client method?

and, are we explicitly stating that, outside the tbdex protocol layer (maybe at a UI / application layer) is where we figure out the amount that comes out of Alice's pocket which becomes rfq.payinUnits ?

regardless of how she asked (say, in USD > BTC txn) i want to receive this much BTC excluding / including the payout fee or i want to pay this much USD excluding / including the payin fee?

diehuxx commented 11 months ago

Adding some good points by @andresuribe87 mentioned in #203. I like the idea of creating a Decimal string type.

https://github.com/TBD54566975/tbdex/issues/203#issue-2033351252

Many fields use a data type of string to represent numbers. The description of the fields mention an "amount". Some examples include:

  • Offering.payoutUnitsPerPayinUnit
  • CurrencyDetails.currencyCode
  • CurrencyDetails.minSubunits

It would be helpful to clarify what possible values are for those fields. One proposal below.

Define a data type that is Decimal. The data type Decimal is serialized as a string. The only possible values of a string such that it would be correctly deserialized would be values that match the regex /^\d*\.?\d*$/ (or similar).

mistermoe commented 11 months ago

During tbdex office hours we decided:

  1. string amounts
  2. major units
  3. decimal separator

re: field names

names suggested in this comment seem to be good with everyone. favor *Amount for the fields that had choices

diehuxx commented 9 months ago

Spec change: https://github.com/TBD54566975/tbdex/pull/206 Javascript implementation change: https://github.com/TBD54566975/tbdex-js/pull/134 Kotlin implementation change: https://github.com/TBD54566975/tbdex-kt/pull/129