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

Offers #798

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Offers #798

wants to merge 16 commits into from

Conversation

rustyrussell
Copy link
Collaborator

@rustyrussell rustyrussell commented Aug 31, 2020

Offers are "static invoices", with many more features. This is mostly implemented in c-lightning already, and is now ready for wider testing.

This PR is based on onion messages specified in #759

04-onion-routing.md Outdated Show resolved Hide resolved
04-onion-routing.md Outdated Show resolved Hide resolved
04-onion-routing.md Outdated Show resolved Hide resolved
09-features.md Outdated Show resolved Hide resolved
13-offer-encoding.md Outdated Show resolved Hide resolved
13-offer-encoding.md Outdated Show resolved Hide resolved
13-offer-encoding.md Outdated Show resolved Hide resolved
13-offer-encoding.md Outdated Show resolved Hide resolved
13-offer-encoding.md Outdated Show resolved Hide resolved
13-offer-encoding.md Outdated Show resolved Hide resolved
Copy link
Contributor

@dr-orlovsky dr-orlovsky left a comment

Choose a reason for hiding this comment

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

Gave an initial read, really like the concept.

With the new developments in miniscript and descriptors the need for invoices is more generic than simple LN: maybe some common format solving bech23 and miniscript use problems as well?

As you probably remember, I am working on RGB: assets & smart contracts over bitcoin and LN with Peter Todd client-side validation paradigms (not tx space use, pure L2/L3) backed by Bitfinex and USDT. In this regard we need shared extensible multi-asset invoice format, preferably with confidential asset support as well.

01-messaging.md Outdated Show resolved Hide resolved
13-offer-encoding.md Outdated Show resolved Hide resolved
13-offer-encoding.md Outdated Show resolved Hide resolved
13-offer-encoding.md Outdated Show resolved Hide resolved
13-offer-encoding.md Outdated Show resolved Hide resolved
11-payment-encoding.md Outdated Show resolved Hide resolved
07-routing-gossip.md Outdated Show resolved Hide resolved
13-offer-encoding.md Outdated Show resolved Hide resolved
@rustyrussell rustyrussell marked this pull request as ready for review April 16, 2021 05:52
@rustyrussell rustyrussell changed the title DRAFT: Offers Offers Apr 16, 2021
@rustyrussell
Copy link
Collaborator Author

I have merged the million-fixup-commits into one commit, and trivially rebased onto the updated #759.

The last two commits are new:

  1. Make spelling happy.
  2. Add a method to replace invoices; I've not yet implemented this though!

@renepickhardt
Copy link
Contributor

I have a question with respect to replacing invoices and the fact that we can proof if the payee claimed both htlcs:

What is the use of that? As far as I understand we could sue the person but this won't give us a cryptographic guarantee to get our money back as even a convicted person might have run or just have already spent the money. Or do I miss something?

The only fix that cryptographically would give us a guarantee to not pay twice that I can think of only seems to shift the problem of stuck payments (and would thus create a burden to the network) and is the following:

  • Assume Alice sends out an onion to Bob over some path that gets stuck
  • Alice wants to use your replacement mechanism to cancle the payment.
  • before (!) sending out a new onion with the new payment hash alice waits for an incoming htlc of the old payment hash (this assumes that Bob after sending out the replacement invoice sets a "reimbursement path" to Alice.

Now we have 2 scenarios:

  1. The orinal payment times out and gets unstuck. Alice would also cancle the reimbursement payment (of course opening Bob to a stuck payment attack from Alice or actually from anyone else when he sets up the reimbursement path)
  2. The original onion arrives at Bob. He should fail the onion and if so Alice should fail the reimbursement onion once her initial htlc is withdrawn or if Bob claims the funds Alice can use the preimage on the reimbursement path to claim her money back.

As said initially: I don't think the reimbursement path (which is by the way inspired by the boomerang paper c.f.: https://arxiv.org/abs/1910.01834 ) solves any issue in our case because the reimbursement path is also open to stuck payment attacks and overall we bind liquidity in two directions now.

{
"description": "with blinded path via Bob (0x424242...), blinding 020202...",
"valid": true,
"bolt12": "lno1pgx9getnwss8vetrw3hhyucs5ypjgef743p5fzqq9nqxh0ah7y87rzv3ud0eleps9kl2d5348hq2k8qzqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgqpqqqqqqqqqqqqqqqqqqqqqqqqqqqzqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqqzq3zyg3zyg3zyg3vggzamrjghtt05kvkvpcp0a79gmy3nt6jsn98ad2xs8de6sl9qmgvcvs",
Copy link

@jkczyz jkczyz Apr 22, 2024

Choose a reason for hiding this comment

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

Here is the same blinded path using an sciddir introduction node:

lno1pgx9getnwss8vetrw3hhyucs3yqqqqqqqqqqqqp2qgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqqyqqqqqqqqqqqqqqqqqqqqqqqqqqqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqqgzyg3zyg3zyg3z93pqthvwfzadd7jejes8q9lhc4rvjxd022zv5l44g6qah82ru5rdpnpj

Comment on lines 648 to 657
- if the invoice is in response to an `invoice_request`:
- MUST copy all non-signature fields from the `invoice_request` (including unknown fields).
- if `invreq_amount` is present:
- MUST set `invoice_amount` to `invreq_amount`
- otherwise:
- MUST set `invoice_amount` to the *expected amount*.
- otherwise (invoice not requested, e.g. for user to scan directly):
- MUST set `invreq_chain` as it would for an invoice_request.
- MUST set `offer_description` as it would for an offer.
- MUST NOT set `invreq_payer_id` or `offer_node_id`.
Copy link

Choose a reason for hiding this comment

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

Just noticing that we don't copy "all non-signature fields" for the "refund" variation. Why is this the case? In LDK we use this data to determine if we should pay the invoice.

Copy link

@jkczyz jkczyz Apr 23, 2024

Choose a reason for hiding this comment

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

@rustyrussell Oh, wait this is talking about a scanned invoice. Shouldn't this be removed now?

Add example of scid first-id for blinded path (thanks jkczyz!)
@rustyrussell
Copy link
Collaborator Author

Can you explain the statement if it uses *offer_amount* to provide the user with a cost estimate? I have no idea why there is any "estimating" happening here. Why is everything not exact?

Offer can be in a different currency. invoice will be in sats.

@rustyrussell
Copy link
Collaborator Author

Can you explain the difference between invreq_amount, invreq_quantity, and offer_amount? This is completely ambiguous in the spec.

I've added the following to the rationale:

Because offer_amount can be in a different currency (using the offer_currency field) it is merely a guide: the issuer will convert it into a number of millisatoshis for invoice_amount at the time generates an invoice, or the invoice_request can specify the exact amount, but the issuer may then reject it if it disagrees.

?

@rustyrussell
Copy link
Collaborator Author

How is "seller generates a refund invoice request each time it generates an invoice and sends both together as a reply to the buyer's invoice request" possible with this spec? I asked for something similar in #798 (comment) , but I thought the idea was rejected for now.

Yeah, while you could support this, I want to master the simpler flow first.

Still, why can't a fixed invoice QR code be used for a refund right now though? What TLV field in the invoice request is going to be variable? The only thing I can think of is the expiry, which I could set far out into the future.

The current flow is as primitive as I could make it. Bob refunds Alice by showing her an invoice_request, which refers to the original invoice Alice paid. She sends an invoice, which proves she was the one who actually made the payment (technically: same key she signed the invoice request originally), and gets paid.

Two future extensions are interesting which would automate this over the network:

  1. Bob just tries, best-effort, to send the invoice_request to wherever he sent the original invoice.
  2. Alice just tries, best-effort, to ask Bob for a refund on a previous payment by sending an invoice.

@rustyrussell
Copy link
Collaborator Author

With that being said, I am wondering if the quantity of items you want to buy is out of scope. You are assuming here that the buyer wants to only buy multiple of the same product, but what if they want to buy many different types of products? It seems like that the type, quantity, and unit price of all products should be included in some passive, and more flexible metadata description of what the invoice is for. The node should only care about the total invoice amount and nothing else. I think that the invreq_quantity should always be 1 and should be eliminated.

I think a similar point was made earlier: #798 (comment)

I'd imagine a more complicated metadata description would blow-up the offer QR code size. You're more likely to have a dynamically generated offer for each customer order that doesn't use quantity at all but rather represents the full order from whatever was in the customer's shopping cart. Kinda makes me wonder what the compelling uses for quantity are.

Many things are bought in quantity? Of course, combining multiple offers into a single invoice_request -> invoice would be a nice optimization, which we can do in future too (every wallet becomes a shopping cart!), but this the MVP to allow a single QR per product, rather than having to have a checkout process which generates it dynamically?

Two cases I think of are flour per 100g, or years of domain name registration.

Remove leftover reference to direct invoices (thanks jkczyz!)
describe the offer_amount/currency/invreq_amount/invoice_amount purpose
make offer_node_id optional, if they specify offer_paths.

1. We now tell them explicitly where to send the onion message.
2. We use this as shorthand later when they're supposed to check the reply.
3. Raw invoice_requests cannot set `offer_paths`, so we add `invreq_paths` for this.

Thanks to thomash-acinq and jkczyz!
Make offer_description optional if there's no amount; this is the common "tip" case.

Note that this implies a minimal offer is simply a bech32 encoding of the offer_node_id field, or "lno1" + bech32(0x2242<node_id>).  This might become normalized as the "default offer" to which you can throw sats to a node directly?
@rustyrussell
Copy link
Collaborator Author

@thomash-acinq and @jkczyz I have done those, plus as Matt wanted, made the description optional (iff there's no amount). This makes the smallest possible offer lno1 + bech32(0x2242<node_id>). If nodes start making that offer by default, it would allow you to spray sats to nodes by node id by requesting invoices.

@AndySchroder
Copy link

Can you explain the difference between invreq_amount, invreq_quantity, and offer_amount? This is completely ambiguous in the spec.

I've added the following to the rationale:

Because offer_amount can be in a different currency (using the offer_currency field) it is merely a guide: the issuer will convert it into a number of millisatoshis for invoice_amount at the time generates an invoice, or the invoice_request can specify the exact amount, but the issuer may then reject it if it disagrees.

?

What is the purpose of referencing other currencies? Seems to provide wasted space in the spec and confusion when sat is the global standard.

@AndySchroder
Copy link

Can you explain the statement if it uses *offer_amount* to provide the user with a cost estimate? I have no idea why there is any "estimating" happening here. Why is everything not exact?

Offer can be in a different currency. invoice will be in sats.

Again, referencing unrelated currencies seems to provide wasted space and confusion.

Comment on lines +308 to +309
- if `offer_node_id` is set:
- MUST send the onion message to `offer_node_id`
Copy link

@jkczyz jkczyz Apr 24, 2024

Choose a reason for hiding this comment

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

Given the writer MAY set offer_node_id to the node's public key when offer_paths is set, the reader can't assume it is the node's public key -- it may be transient! So readers should only send to offer_node_id when offer_paths is not set.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This seems very wrong to me. If you don't want them prove a offer_node_id, don't set it. But offer_paths ids are forced transient: they are useless for any reputation system (they sign the invoice with that random key, but that means nothing).

Public nodes will want to sign their invoices with their node_id, so will any vendors trying to build a reputation for honesty. See Matt's DNS proposal, for example. Such offers may also want to use blinded paths, as they're the only way to give capacity or routing hints.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I strongly disagree that we should be continuing to tie the node_id concept to the thing that signs invoices, in fact the separation is one reason I'm excited by offers. A single merchant may have several nodes, but more importantly we really should be separating the key material that is signing proof-of-payment data from the key material that is being used to decode onions or establish P2P connections. Reusing the same key material across so many different contexts is just horrible practice.

Choose a reason for hiding this comment

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

The reason for setting both offer_paths and offer_node_id would be to enjoy the privacy of blinded paths while still using a stable identity that user can attach a reputation to.
Example: I'm a merchant and I want to use a stable node id that user can learn to trust, so I set offer_node_id and sign invoices with it. However I either don't have a public node (e.g. phoenix) or don't want it to be known that it is my node, so I use offer_paths and set offer_node_id to be different from the node id of my lightning node.
I also disagree with your assertion that blinded paths are necessarily transient, all phoenix wallets will use blinded paths that will be valid forever.

@jkczyz
Copy link

jkczyz commented Apr 24, 2024

What is the purpose of referencing other currencies? Seems to provide wasted space in the spec and confusion when sat is the global standard.

You wouldn't want a subscription / recurring offer denominated in msats if the price of BTC is trending up. Currencies were largely for that purpose though recurrences were removed from v1 of the spec.

@jkczyz
Copy link

jkczyz commented Apr 24, 2024

@thomash-acinq and @jkczyz I have done those, plus as Matt wanted, made the description optional (iff there's no amount). This makes the smallest possible offer lno1 + bech32(0x2242<node_id>). If nodes start making that offer by default, it would allow you to spray sats to nodes by node id by requesting invoices.

@TheBlueMatt was thinking the description could be always optional. Any reason why it is required when there is an amount?

@AndySchroder
Copy link

What is the purpose of referencing other currencies? Seems to provide wasted space in the spec and confusion when sat is the global standard.

You wouldn't want a subscription / recurring offer denominated in msats if the price of BTC is trending up. Currencies were largely for that purpose though recurrences were removed from v1 of the spec.

Seems like it makes sense to have removed recurrences from the spec. I think you should always request a new invoice if some time has passed, all markets can always shift. If there are no more recurrences, does it make sense to remove arbitrary currencies from the spec too? It seems to me like if you are basing your prices on some arbitrary currency, that should be done out of band.

@jkczyz
Copy link

jkczyz commented Apr 24, 2024

Seems like it makes sense to have removed recurrences from the spec. I think you should always request a new invoice if some time has passed, all markets can always shift. If there are no more recurrences, does it make sense to remove arbitrary currencies from the spec too? It seems to me like if you are basing your prices on some arbitrary currency, that should be done out of band.

I'm indifferent to removing it now. Note that the currency is only in offer not the invoice. For a recurring payment, the user would request a new invoice each time.

@thomash-acinq
Copy link

I think you should always request a new invoice if some time has passed, all markets can always shift.

Invoices are single-use, you must always request a new invoice each time you want to make a payment regardless of how much time has passed.

If there are no more recurrences, does it make sense to remove arbitrary currencies from the spec too? It seems to me like if you are basing your prices on some arbitrary currency, that should be done out of band.

Some day in the future, goods may be priced in bitcoins, but today merchants set their prices in euros, dollars or other fiat currencies, that's the reality we live in. If we want real life merchants to use offers, they must support fiat currencies.

@rustyrussell
Copy link
Collaborator Author

What is the purpose of referencing other currencies? Seems to provide wasted space in the spec and confusion when sat is the global standard.

Real charges are still in other currencies, and will be for the forseeable future. However, if no currency is specified, it's in (milli)sats.

@rustyrussell
Copy link
Collaborator Author

@thomash-acinq and @jkczyz I have done those, plus as Matt wanted, made the description optional (iff there's no amount). This makes the smallest possible offer lno1 + bech32(0x2242<node_id>). If nodes start making that offer by default, it would allow you to spray sats to nodes by node id by requesting invoices.

@TheBlueMatt was thinking the description could be always optional. Any reason why it is required when there is an amount?

It only makes sense to omit it when there's no specific purpose for the payment. Otherwise, I really want you indicate what it is you think I'm paying for! This is approximately maps to "specific amount: specific purpose". It's a compromise, which allows for a tiny qr / default offer for a simple "throw sats at this node".

Comment on lines +508 to +511
"description": "Missing offer_description",
"valid": false,
"bolt12": "lno1zcss9mk8y3wkklfvevcrszlmu23kfrxh49px20665dqwmn4p72pksese"
},
Copy link

Choose a reason for hiding this comment

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

Looks like this test vector needs to be updated to include an amount now.

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.

None yet