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

BOLT 12: remove send_invoice offer, use a straight invoice_request. #11

Closed
wants to merge 3 commits into from

Conversation

rustyrussell
Copy link
Owner

Instead of a kind of dummy offer which asks you to send an invoice,
use a dummy invoice request, since it now contains all the offer
fields anyway. It simplifies things a little, but there's the
following fallout:

  1. We use the presence of offer_node_id to distinguish
    invoice-request-for-offer and invoice-request-no-offer, which makes
    sense, since the vendor doesn't know that.
  2. We rename invoice_request_payer_key to invoice_request_payer_id. In
    the case of a public node making a non-offer invoice_request, it's
    their id (as they are the payer), but it can omitted using blinded
    paths (like the offer_node_id in offer).
  3. The invoice_request has no signature if it's not an offer response.
    This is makes them as small like the old send_invoice offer.
  4. We don't allow invoice_request in other currencies (could in future):
    must specify in msats (this issue is expanded in the error section).
  5. Actually tell them they're supposed to request an invoice when they
    receive an invoice_request for an offer! (They don't have to if it's
    a non-offer one, they would be expected to check with user).
  6. No signatures on invoices if they're non-offer, but allow a verification
    code. If the user presents an offer to the ATM in the first place, this
    can be skipped, but requires it have a camera, and more interaction.

Here are examples of the offer-request-invoice flow, and the
request-invoice flow:

offer-request-invoice: Rusty buys a coffee from Starblocks

  1. Scans lno...

    • offer_description: 1 cup coffee.
    • offer_amount: 1000000msat
    • offer_issuer: Starblocks starblocks.acinq.co
    • offer_node_id: starblocks-nodeid
  2. Rusty sends onionmessage to starblocks-nodeid with reply_path back Rusty, and
    invoice_request:

    • invoice_request_payer_info: 23456...
    • offer_description: 1 cup coffee.
    • offer_amount: 1000000msat
    • offer_issuer: Starblocks starblocks.acinq.co
    • offer_node_id: starblocks-nodeid
    • invoice_request_payer_id: rusty-tmpkey
    • invoice_request_payer_note: love that coffee!
    • signature: SIG(^^, rusty-tmpkey)
  3. Starblocks replies with invoice:

    • invoice_request_payer_info: 23456...
    • offer_description: 1 cup coffee.
    • offer_amount: 1000000msat
    • offer_issuer: Starblocks starblocks.acinq.co
    • offer_node_id: starblocks-nodeid
    • invoice_request_payer_id: rusty-tmpkey
    • invoice_request_payer_note: love that coffee!
    • invoice_paths: 1 dummy-path
    • invoice_blindedpay: 1 dummy-payinfo
    • invoice_created_at: now
    • invoice_payment_hash: 34567...
    • signature: SIG(^^, offer_node_id)

request-invoice: Rusty buys 1000 sats from Matt

  1. Matt generates invoice request (lnr...)

    • offer_description: 25c worth of bitcoin
    • offer_issuer: BlueMatt
    • invoice_request_payer_id: matt-nodeid
  2. Rusty scans this, sends onionmessage to matt-nodeid... with reply_path back Rusty (for errors). App shows the random invoice code for validation.

    • offer_description: 25c worth of bitcoin
    • offer_issuer: BlueMatt
    • invoice_request_payer_id: matt-nodeid
    • invoice_paths: At least 1 path
    • invoice_blindedpay: As many payinfo as invoice_path
    • invoice_created_at: now
    • invoice_payment_hash: 34567...
    • invoice_code: AV2PEE
  3. Matt doesn't bother checking the invoice code and just hits "OK
    pay", because Rusty's right next to him and he assumes nobody
    intercepted the QR code.

Signed-off-by: Rusty Russell [email protected]

Copy link
Collaborator

@valentinewallace valentinewallace left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me! send_invoice did previously feel a little shoved in, it feels more fleshed out this way? Would need Jeff's take for which is more encoding-friendly.

12-offer-encoding.md Outdated Show resolved Hide resolved
12-offer-encoding.md Outdated Show resolved Hide resolved
12-offer-encoding.md Show resolved Hide resolved
@valentinewallace
Copy link
Collaborator

valentinewallace commented Aug 24, 2022

After giving this another glance, my one doubt is that it seems like invoice_requests contain strictly more fields and thus may be harder to QR scan than the original Offer with simply send_invoice set, IIUC

@jkczyz
Copy link

jkczyz commented Aug 24, 2022

After giving this another glance, my one doubt is that it seems like invoice_requests contain strictly more fields and thus may be harder to QR scan than the original Offer with simply send_invoice set, IIUC

IIUC, the QR code would be for the invoice_request in the second example, which would be comparable in size to an offer. The first example is the typical flow where only the offer is communicated via QR code.

@valentinewallace
Copy link
Collaborator

After giving this another glance, my one doubt is that it seems like invoice_requests contain strictly more fields and thus may be harder to QR scan than the original Offer with simply send_invoice set, IIUC

IIUC, the QR code would be for the invoice_request in the second example, which would be comparable in size to an offer. The first example is the typical flow where only the offer is communicated via QR code.

Hmm ok, looking in more detail it seems like it depends how you implement it.
It looks like offers have offer_currency and an extra *_quantity_* field that invoice_requests don't have, and invoice_requests have invoice_request_payer_info and invoice_request_payer_note that offers don't have.

So you could leave offer_metadata and invoice_request_payer_note blank, and they would be ~the same size with invoice_request being a bit smaller. But if we'll need to set offer_metadata, then offers are nontrivially smaller. Maybe something to discuss further.

Sorry I can't paste this with formatting but here's how I drew out the fields:

Screen Shot 2022-08-24 at 12 38 34 PM

@jkczyz
Copy link

jkczyz commented Aug 24, 2022

So you could leave offer_metadata and invoice_request_payer_note blank, and they would be ~the same size with invoice_request being a bit smaller. But if we'll need to set offer_metadata, then offers are nontrivially smaller. Maybe something to discuss further.

That's a good point. May be useful to calculate how large a typical invoice_request would be in this use case compared to what maximum QR code size a typical camera could support. Presumably, some fields like invoice_request_chain typically would be inferred rather than explicitly set. But fields like offer_metadata may well be needed for another 55 bytes assuming a 32-byte HMAC (1-byte type, 1-byte length, 32 byte value, times 1.6 for bech32).

- if it supports bolt12 features:
- MUST set `invoice_request_features`.`features` to the bitmap of features.

The reader:
- MUST fail the request if `invoice_request_payer_key` is not present.
- MUST fail the request if `invoice_request_payer_id` is not present.
Copy link
Collaborator

@valentinewallace valentinewallace Aug 25, 2022

Choose a reason for hiding this comment

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

@rustyrussell This shouldn't hold if the invoice is in response to an inv_req QR, right? I believe the invoice_request_payer_id is optional in that case since there's no signature (assuming blinded paths are present)?

@jkczyz if this is correct, then I think the resolution of #11 (comment) is that we'll never set these fields for QRs and always just set offer_metadata, so invoice_request ends up being a bit smaller than offer_send_invoices but comparable

Copy link
Owner Author

Choose a reason for hiding this comment

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

I ended up dropping the whole "node_id is optional if you use a blinded path and don't have a real node id" since it just made this complicated. So that would apply here too, and it might be important later if you want to later prove your id? It's certainly harmless to include one.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmmm, aren't we trying to save bits on the QR code here? If I'm missing something, though, I think the requirements for the writer need updating for this in the QR case

Copy link
Owner Author

Choose a reason for hiding this comment

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

Once you have blinded paths in the QR code, it's hard to argue that saving 35 bytes is a big win. It's possible to allow this case later, but for now we have more than enough complexity :(

Copy link
Collaborator

Choose a reason for hiding this comment

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

35 bytes is pretty significant in a QR code, plus even a single 2-hop blinded path can add a lot of privacy so I'm not convinced that "blinded paths == we should give up entirely on the QR case" IMHO.

Not gonna die on this hill but wanted to put out those points :p it's true we can add it later as you say

Copy link

Choose a reason for hiding this comment

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

FWIW, it's actually 56 bytes after bech32 encoding, IIUC.

12-offer-encoding.md Outdated Show resolved Hide resolved
12-offer-encoding.md Show resolved Hide resolved
2. The user sends an *invoice* for the amount in the *offer*
3. The merchant makes a payment to the user indicated by the invoice.
1. The merchant publishes an *invoice_request* which contains offer fields
which refer to its attempt to send money, including an amount.
Copy link

Choose a reason for hiding this comment

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

nit: s/which/that

12-offer-encoding.md Show resolved Hide resolved
12-offer-encoding.md Show resolved Hide resolved
Instead of a kind of dummy offer which asks you to send an invoice,
use a dummy invoice request, since it now contains all the offer
fields anyway.  It simplifies things a little, but there's the
following fallout:

1. We use the presence of `offer_node_id` to distinguish
   invoice-request-for-offer and invoice-request-no-offer, which makes
   sense, since the vendor doesn't know that.
2. We rename `invoice_request_payer_key` to `invoice_request_payer_id`.  In
   the case of a public node making a non-offer invoice_request, it's
   their id (as they are the payer), but it can omitted using blinded
   paths (like the offer_node_id in offer).
3. The invoice_request has no signature if it's not an offer response.
   This is makes them as small like the old send_invoice offer.
4. We don't allow invoice_request in other currencies (could in future):
   must specify in msats (this issue is expanded in the error section).
5. Actually tell them they're supposed to request an invoice when they
   receive an `invoice_request` for an offer!  (They don't have to if it's
   a non-offer one, they would be expected to check with user).
6. No signatures on invoices if they're non-offer, but allow a verification
   code.  If the user presents an offer to the ATM in the first place, this
   can be skipped, but requires it have a camera, and more interaction.

Here are examples of the offer-request-invoice flow, and the
request-invoice flow:

### offer-request-invoice: Rusty buys a coffee from Starblocks

1. Scans lno...

   * offer_description: 1 cup coffee.
   * offer_amount: 1000000msat
   * offer_issuer: Starblocks starblocks.acinq.co
   * offer_node_id: starblocks-nodeid

2. Rusty sends onionmessage to starblocks-nodeid with reply_path back Rusty, and
invoice_request:

   * invoice_request_metadata: 23456...
   * offer_description: 1 cup coffee.
   * offer_amount: 1000000msat
   * offer_issuer: Starblocks starblocks.acinq.co
   * offer_node_id: starblocks-nodeid
   * invoice_request_payer_id: rusty-tmpkey
   * invoice_request_payer_note: love that coffee!
   * signature: SIG(^^, rusty-tmpkey)
   
3. Starblocks replies with invoice:

   * invoice_request_metadata: 23456...
   * offer_description: 1 cup coffee.
   * offer_amount: 1000000msat
   * offer_issuer: Starblocks starblocks.acinq.co
   * offer_node_id: starblocks-nodeid
   * invoice_request_payer_id: rusty-tmpkey
   * invoice_request_payer_note: love that coffee!
   * invoice_paths: 1 dummy-path
   * invoice_blindedpay: 1 dummy-payinfo
   * invoice_created_at: now
   * invoice_payment_hash: 34567...
   * signature: SIG(^^, offer_node_id)

### request-invoice: Rusty buys 1000 sats from Matt

1. Matt generates invoice request (lnr...)

   * offer_description: 25c worth of bitcoin
   * offer_issuer: BlueMatt
   * invoice_request_payer_id: matt-nodeid

2. Rusty scans this, sends onionmessage to matt-nodeid... with reply_path back Rusty (for errors).  App shows the random invoice code for validation.

   * offer_description: 25c worth of bitcoin
   * offer_issuer: BlueMatt
   * invoice_request_payer_id: matt-nodeid
   * invoice_paths: At least 1 path
   * invoice_blindedpay: As many payinfo as invoice_path
   * invoice_created_at: now
   * invoice_payment_hash: 34567...
   * invoice_code: AV2PEE
 
3. Matt doesn't bother checking the invoice code and just hits "OK
   pay", because Rusty's right next to him and he assumes nobody
   intercepted the QR code.

Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
Most confusingly, the "if `offer_amount` is present" case was
exactly backwards, which doesn't make sense at all.

Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell
Copy link
Owner Author

Merged into bolt12 PR. Thanks!

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.

3 participants