Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add proposal for currencies on payRequest #251

Open
wants to merge 12 commits into
base: luds
Choose a base branch
from

Conversation

lsunsi
Copy link
Contributor

@lsunsi lsunsi commented Dec 1, 2023

Abstract

This PR proposes an extension to the payRequest specification to allow for denomination of amount in different currencies and negotiating a conversion to this currency. It aims to be backwards compatible while taking a further step on providing lightning wallets and services a common language about exchanges and remittances on foreign currencies.

The PR includes what I expect to be a detailed description with some examples of the proposed extension. It also includes some references to related works that can be competing or complementary proposals to this one. This proposal is inspired by those references.

Implementations

I've been discussing this proposal inside @bipa-app currently and I aim to implement the most recent version of it and testing with partners before merging this PR.

EDIT:

Current proposal implemented at lsunsi@bipa.app if anyone wants to check it out.

@lsunsi lsunsi force-pushed the add-lud-currencies branch 2 times, most recently from 6fceacf to 35ceb75 Compare December 1, 2023 14:25
@lsunsi lsunsi marked this pull request as ready for review December 1, 2023 17:00
@jklein24
Copy link

jklein24 commented Dec 2, 2023

Nice :-D. As discussed on telegram, it would be great to align this with the existing LUD-21 proposal (and UMA) where possible. UMA actually already extended that proposal in a few of the ways you've done here:

  1. Using multiple currencies in the first response.
  2. Locking in the exchange rate implicitly via invoice expiration time (see https://arc.net/l/quote/nbilkwwa)
  3. Including displayDecimals in the Currency object (would be nice to align on naming).

A few super simple changes to make these more consistent:

  1. Change the name and semantics of price to match multiplier in the existing spec
  2. Use displayDecimals instead of decimals
  3. Use an array instead of an object for currencies and add the code field to the currency. The main reason for this is that it allows a sense of ordering for preferences by the payee. If I prefer BRL, but can also receive BTC, I'm able to set that preference via the ordering within the array.
  4. Optional: include the max and min sendable fields from the other proposal.

Also a very minor spelling nit - convertable -> convertible.

21.md Outdated Show resolved Hide resolved
21.md Outdated Show resolved Hide resolved
21.md Outdated Show resolved Hide resolved
21.md Outdated Show resolved Hide resolved
21.md Outdated Show resolved Hide resolved
@lsunsi lsunsi force-pushed the add-lud-currencies branch from 35ceb75 to 7b19a21 Compare December 3, 2023 12:53
@lsunsi
Copy link
Contributor Author

lsunsi commented Dec 3, 2023

@jklein24 Some comments on your

Change the name and semantics of price to match multiplier in the existing spec

I have no problems making this change, but would you explain to me why it's better? It feels to me like multiplier makes sense on the context of @ethanrose proposal, but in mine it can even hinder understanding (since you have to multiply it if you input the value in currency, but you have to divide by it in case you input in BTC). Again, I'm just trying to understand better, because readon 38000 in this field might be easier to understand than 2500000 (in case of USD,

Use displayDecimals instead of decimals

Changed!

Use an array instead of an object for currencies and add the code field to the currency. The main reason for this is that it allows a sense of ordering for preferences by the payee. If I prefer BRL, but can also receive BTC, I'm able to set that preference via the ordering within the array.

This is really interesting. Thinking about a little, how would you say the preference is BTC if BTC is not listed in the currencies? Moreover, maybe this could be achieved with a separate preferredCurrency field or something like that? I really like this idea, just thinking of the best way to achieve it.

Optional: include the max and min sendable fields from the other proposal.

I thought about that as well, but thought that maybe it could be an extension of the proposal, since the max/min already present kind of fulfills this role? What do you think? I'd love to hear what @ethanrose thinks as well.

Also a very minor spelling nit - convertable -> convertible.

Not very minor, cmon, I'd hate to merge a spelling mistake into this repo haha thanks! fixed

@lsunsi lsunsi force-pushed the add-lud-currencies branch from 7b19a21 to 56bbdd8 Compare December 3, 2023 13:01
@jklein24
Copy link

jklein24 commented Dec 4, 2023

This is really interesting. Thinking about a little, how would you say the preference is BTC if BTC is not listed in the currencies? Moreover, maybe this could be achieved with a separate preferredCurrency field or something like that? I really like this idea, just thinking of the best way to achieve it.

The order of currencies in the array dictates order of preferences between currencies. So if I prefer BRL, but can also get BTC, you'd have your currencies array contain BRL first and then BTC. You can check out the UMA protocol to see the full structure, but essentially it would look like:

"currencies": [
    {
      "code": "BRL",
      "name": "Brazilian Real",
      "symbol": "R$",
      "minSendable": 1,
      "maxSendable": 1_000_000,
      // Estimated millisats per "unit" (eg. 1 cent in USD)
      "multiplier": 489,
      // Number of digits after the decimal point for display on the sender side. For example,
      // in USD, by convention, there are 2 digits for cents - $5.95. in this case, `displayDecimals`
      // would be 2. Note that the multiplier is still always in the smallest unit (cents). This field
      // is only for display purposes. The sender should assume zero if this field is omitted, unless
      // they know the proper display format of the target currency.
      "displayDecimals": 2,
    },
    {
      "code": "SAT",
      "name": "Satoshis",
      "symbol": "sat",
      "minSendable": 1,
      "maxSendable": 10_000_000,
      "multiplier": 1_000, // estimated millisats per "unit" (eg. 1 cent in USD)
      "displayDecimals": 0,
    },
  ],

@jklein24
Copy link

jklein24 commented Dec 4, 2023

I have no problems making this change, but would you explain to me why it's better? It feels to me like multiplier makes sense on the context of @ethanrose proposal, but in mine it can even hinder understanding (since you have to multiply it if you input the value in currency, but you have to divide by it in case you input in BTC). Again, I'm just trying to understand better, because reading 38000 in this field might be easier to understand than 2500000 (in case of USD,

Would love @ethanrose's take on this too. Tbh I wasn't thinking it was better for some technical reason, but more just because so many entities are already live with the multiplier name, so staying consistent would be nice. It sounds like maybe I'm still missing some difference between price and multiplier though (besides just the exponent). I wasn't thinking you'd need to change whether you multiply or divide based on the input currency, but rather that it's just always the multipler from your input currency to mSats. What am I missing there?

@lsunsi
Copy link
Contributor Author

lsunsi commented Dec 4, 2023

@jklein24 Maybe I am the one missing something!

Let me try to explain.
Considering the price is 200000 and multiplier is 500000, consider this example.

First example

The user inputs that it wants to send 1 BRL to the other side.
The request equivalent of this input would be GET bipa.app/callback?amount=1BRL&convert=BRL.

If the WALLET wants to preview (before calling the callback) what the amount of the invoice would be, what should it do?
If it has a price, it needs to approximate amount = 1 / price * 1e3 in millisatoshis.
If it has a multiplier, it needs to approximate amount = multiplier * 1 in millisatoshis.

This is the use-case supported by @ethanrose PR and it looks nicer having the multiplier in this case.

Second example

The user inputs that it wants to send 500 sats as BRL to the other side.
The request equivalent of this inputGET bipa.app/callback?amount=500000&convert=BRL

If the WALLET wants to preview (before calling the callback) what the amount reaching the destination, what should it do?
If it has a price, it needs to approximate amount = 500 / 1e8 * price in BRL.
If it has a multiplier, it needs to approximate amount = 500 * 1e3 / multiplier in BRL.

Conclusion

In the second case you use the multiplier as the denominator, which I think can be confusing this it's called "multiplier".
Both cases work of course, because price and multiplier are equivalent, my only question is about the name "multiplier" being confusing, but maybe I'm the only one getting confused haha.

Does these examples help in any way clear up the confusion?
Also, since you first proposed the multiplier field, maybe you want to weight in on what do you prefer for this proposal too @fiatjaf

@jklein24
Copy link

jklein24 commented Dec 4, 2023

@jklein24 Maybe I am the one missing something!

Let me try to explain. Considering the price is 200000 and multiplier is 500000, consider this example.

First example

The user inputs that it wants to send 1 BRL to the other side. The request equivalent of this input would be GET bipa.app/callback?amount=1BRL&convert=BRL.

If the WALLET wants to preview (before calling the callback) what the amount of the invoice would be, what should it do? If it has a price, it needs to approximate amount = 1 / price * 1e3 in millisatoshis. If it has a multiplier, it needs to approximate amount = multiplier * 1 in millisatoshis.

This is the use-case supported by @ethanrose PR and it looks nicer having the multiplier in this case.

Second example

The user inputs that it wants to send 500 sats as BRL to the other side. The request equivalent of this inputGET bipa.app/callback?amount=500000&convert=BRL

If the WALLET wants to preview (before calling the callback) what the amount reaching the destination, what should it do? If it has a price, it needs to approximate amount = 500 / 1e8 * price in BRL. If it has a multiplier, it needs to approximate amount = 500 * 1e3 / multiplier in BRL.

Conclusion

In the second case you use the multiplier as the denominator, which I think can be confusing this it's called "multiplier". Both cases work of course, because price and multiplier are equivalent, my only question is about the name "multiplier" being confusing, but maybe I'm the only one getting confused haha.

Does these examples help in any way clear up the confusion? Also, since you first proposed the multiplier field, maybe you want to weight in on what do you prefer for this proposal too @fiatjaf

Thanks for the writeup - that makes things much more clear and concrete. I guess the thing is that in your two scenarios, you're asking 2 different questions:

  • Scenario 1: If the WALLET wants to preview (before calling the callback) what the amount of the invoice would be
  • Scenario 2: If the WALLET wants to preview (before calling the callback) what the amount reaching the destination

To me, it makes sense that these different questions would require some different math. In fact for the first scenario, the easiest thing to do would just be to decode the invoice you get back in the pr field, rather than doing the conversion again out-of-band.

@lsunsi
Copy link
Contributor Author

lsunsi commented Dec 5, 2023

@jklein24 I just updated the proposal considering some of the discussed feedback. 55384ff Care to take a look?

21.md Outdated Show resolved Hide resolved
21.md Outdated Show resolved Hide resolved
21.md Outdated Show resolved Hide resolved
21.md Outdated Show resolved Hide resolved
21.md Show resolved Hide resolved
21.md Outdated Show resolved Hide resolved
21.md Outdated Show resolved Hide resolved
@lsunsi lsunsi force-pushed the add-lud-currencies branch 2 times, most recently from 408c732 to 801f196 Compare December 6, 2023 17:14
@lsunsi lsunsi force-pushed the add-lud-currencies branch from 801f196 to 2d37cec Compare December 6, 2023 17:15
21.md Outdated Show resolved Hide resolved
Copy link

@jaonoctus jaonoctus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 7bf8f3a and 2d37cec

code: string, // Code of the currency, used as an ID for it. E.g.: BRL
name: string, // Name of the currency. E.g.: Reais
symbol: string, // Symbol of the currency. E.g.: R$
decimals: number, // Integer; Number of decimal places. E.g.: 2

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: It might be good to include a longer description about this field. With UMA, a bunch of folks integrating have been a bit confused by the mechanics around decimals/multiplier. Might be good to lay out some examples and show why decimals is important for clarity.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI @lsunsi I added some examples and notes on small currency units in the uma spec that might be helpful here too - https://github.com/uma-universal-money-address/protocol/blob/main/umad-04-lnurlp-response.md#currency-examples

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes a lot of sense, I tried complementing the spec with this info and copied your example kind of . What do you think of (6a5b6e0)?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, that captures the gist of it! I think the main difference is that UMA has an explicit max of 8 decimals, but keeping it open is fine too. LGTM

21.md Outdated
{
"pr": "lnbc1230n1pjknkl...ju36m3lyytlwv42fee8gpt6vd2v",
"routes": [],
+ "converted": 123

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Funny enough, in UMA, I chose not to include the target amount, but did include the multiplier and some other stuff:

"paymentInfo": {
    // The currency code of the receiving currency (eg. "USD"). This should match the requested currency in the payreq
    // request.
    "currencyCode": string,
    // Millisats per "unit" of the receiving currency (eg. 1 cent in USD). A double-precision floating point number.
    // In this context, this is just for convenience. The conversion rate is also baked into the invoice amount itself.
    // `invoice amount = amount * multiplier + exchangeFeesMillisatoshi`
    "multiplier": number,
    // Number of digits after the decimal point for the receiving currency. For example, in USD, by convention, there are
    // 2 digits for cents - $5.95. In this case, `decimals` would be 2. This should align with the currency's `decimals`
    // field in the LNURLP response. It is included here for convenience. See [UMAD-04](/uma-04-local-currency.md) for
    // details, edge cases, and examples.
    "decimals": number,
    // The fees charged (in millisats) by the receiving VASP to convert to the target currency.
    // This is separate from the multiplier rate.
    "exchangeFeesMillisatoshi": number
},

The fees are useful for a full visual cost breakdown on the sender side. The multiplier and decimals help infer the converted amount from the invoice amount. I do like having the wrapping struct to keep related fields grouped and allow for extension in the future without polluting the top-level structure. Just some food for thought :-).

UMA spec link: https://github.com/uma-universal-money-address/protocol/blob/main/umad-06-payreq-response.md

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is interesting. The reason I chose to keep just the int is to keep the protocol smaller.
The only property in this structure that is not already included is the fee, right?

The reason I didn't included it is that there's multiple ways of taking the fee, right? It could be billed in sats, or in currency (althought it's kind of the same). But the fee could be in the price (spread) or separate. Etc.

What do you think about these possibilities?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that's fair. I just delimit the fee in mSats always because my assumption is that it's based into the cost of the invoice: invoice amount = amount * multiplier + exchangeFeesMillisatoshi. I like having a top-level structure to wrap payment details since I could imagine more fields being added there in the future for scenarios you're describing. With the wrapper, we wouldn't need to pollute the top level more.

Copy link

@lorenzolfm lorenzolfm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 8c312b4

@lsunsi lsunsi force-pushed the add-lud-currencies branch from 8c312b4 to 7275242 Compare December 27, 2023 16:08
@lsunsi
Copy link
Contributor Author

lsunsi commented Dec 27, 2023

@jklein24 @lorenzolfm @jaonoctus I hacked this webapp to showcase the LUD. It's using just the API described in this PR. https://lnurlp-live.vercel.app/lorenzolfm@bipa.app

21.md Outdated Show resolved Hide resolved
@lsunsi lsunsi force-pushed the add-lud-currencies branch from 7275242 to b5cf042 Compare December 27, 2023 18:58
@lsunsi lsunsi force-pushed the add-lud-currencies branch from f670c07 to f70a0d6 Compare December 28, 2023 12:15
@jaonoctus
Copy link

jaonoctus commented Dec 30, 2023

@lsunsi noice! I wrote a similar interface ¹ to do the samething 😂 that I showed to @lorenzolfm

1

@lsunsi
Copy link
Contributor Author

lsunsi commented Dec 30, 2023

@lsunsi noice! I wrote a similar interface to do the samething 😂 that I showed to @lorenzolfm

Yeah my theory is that he is incepting us with his ideas at the same time!

@jklein24
Copy link

FYI this is now live in UMA v1 and VASPs are rolling it out. Xapo, Ripio, and Bitnob are upgraded and several more are on the way.

jklein24 added a commit to jklein24/nips that referenced this pull request Jul 10, 2024
This will help serve use cases like e-cash wallets, bolt-12 offers which allow for other currency denominations, and [LUD-21](lnurl/luds#251 wallet providers like UMA VASPs.
@lsunsi
Copy link
Contributor Author

lsunsi commented Sep 21, 2024

I'd like to point out that besides being part of UMA v1 like @jklein24 , we have a working implementation at bipa, it's a shame this spec doesn't get more attention from maintainers. Maybe @hsjoberg or @fiatjaf could share some thoughts here!

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.

4 participants