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

Reduce size of offer blinded paths #1099

Closed
wants to merge 10 commits into from

Conversation

jkczyz
Copy link

@jkczyz jkczyz commented Jul 31, 2023

Offers may contain blinded paths to allow for greater recipient privacy. However, they come at a cost of increased QR code size as each hop requires a 33-byte point for the next_node_id. Allow using short_channel_id instead, which only requires 8 bytes. Further, allow for specifying the introduction node as either a point or a short_channel_id in blinded paths via a new node_id fundamental type.

Based on #798.

rustyrussell and others added 9 commits April 3, 2023 09:12
These use onion encoding for simple one-way messaging: there are no error returns.
However, every onion uses route blinding *even if it doesn't need to*.

You can prove what path was used to reach you by including `path_id` in the
encrypted_data_tlv.

Note that this doesn't actually define the payload we're transporting:
that's explictly defined to be payloads in the 64-255 range.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's far easier to validate these on parsing than to hand-validate them
elsewhere.

I didn't turn `alias` or `error` into this, though they're similar
(`alias` can have a nul terminator).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
A BOLT11 "invoice" has proven too low-level for human use in many
scenarios.  Efforts like lnurl have covered the gap, but integrating
some of such higher layers into the lightning protocol itself has many
advantages.

This draft defines three new things:

1. A new invoice format.  I know, this is painful, but it maps almost
   1:1 to the current format (though signatures are very different),
   is easier to implement, and easier to send via the lightning
   network itself.

2. Formats for an "offer", which for all intents and purposes serves
   as the new, persistent invoice for users.

3. Format for an "invoice_request": this is a message sent via the
   lightning network itself to receive the real invoice, or can
   be used directly in a send-money scenario (e.g. ATM).

Features which have been deliberately omitted for the initial version:
- Recurrence.
- Invoice replacement ("don't accept that old payment!")
- Payer proof for refunds.

I need to thank everyone who gave detailed feedback, particularly:

1. Thomas H of ACINQ (https://github.com/thomash-acinq)
2. Joost Jager (https://github.com/joostjager)
3. Aditya Sharma (https://github.com/adi2011)
4. Rene Pickhardt (https://github.com/renepickhardt)
5. Bastien Teinturier (https://github.com/t-bast)
6. Valentine Wallace of LDK (https://github.com/valentinewallace)
7. Matt Corallo of LDK (https://github.com/BlueMatt)
8. Jeffrey Czyz of Square Crypto (https://github.com/jkczyz)

Also @bjarnemagnussen, @ellemouton, @animatedbarber, @617a7a,
@instagibbs, and @eupn.


Header from folded patch 'invoice-features.ptch':

BOLT 12: clarify that feature spaces are different, define invoice MPP feature.

I was going to reuse features, except BOLT 9 says that `basic_mpp` requires
`payment_secret`, which doesn't make sense for BOLT 12.

So let's define a new feature space for each of offers,
invoice_requests and invoices.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>


Header from folded patch 'more-feedback.patch':

fixup! BOLT 12: offers, fifth draft

More clarifications for the sharp-eyed @jkczyz, including changing the
MPP bits in invoices to match bolt 9, for ease.

Others:
1. Remove the (redundant) definition of `blinded_path` from bolt 12,
2. Fix formatting mistake which messed up tools/extract-formats.py.
3. Remove unused `send_invoice` field from invoice.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>


Header from folded patch 'invoice_request-less-verbose.patch':

fixup! BOLT 12: offers, fifth draft

invoice_request_x fields turned into invreq_.  The names get quite long
otherwise, and that's bad for code (which is clearest if it uses the
same names as the spec, even if it's not directly generated).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>


Header from folded patch 'more-fixes-and-quantity-min-remove.patch':

fixup! BOLT 12: offers, fifth draft

1. Remove offer_quantity_min, use offer_quantity_max=0 to indicate any.
2. Renumber offer_node_id so it's still continuous.
3. Reword "any unknown fields > 80" to "any fields > 80".
4. Fixed MPP inline numbers.

Thanks again to @jkczyz.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>


Header from folded patch 'flesh-out-offer-reader-requirements.patch':

fixup! BOLT 12: offers, fifth draft

Make sure every relevant offer field has reader requirements now.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>


Header from folded patch 'fixup-again.patch':

fixup! BOLT 12: offers, fifth draft

1. invoice_request of course sets signature fields (which are greater than 160).
2. invoice_request reader requires invreq_metadata field.
3. Rename "base invoice amount" to clearer "expected amount".
4. MAY FAIL if invoice_request amount "greatly exceeds", to allow tips, etc.
5. Describe how invoices should set `invoice_amount`.
6. Remove bogus exclusion of `invreq_amount` from invoice field comparison (from
   older draft where it could change).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>


Header from folded patch 'update-merkle-test-vectors.patch':

fixup! BOLT 12: offers, fifth draft

Fix test vectors to match new signature scheme.  Now merkle is only used
for the signature, this is really a signature test, so rename the test
vector too.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>


Header from folded patch 'more-trivial-quantity-max-fixes.patch':

fixup! BOLT 12: offers, fifth draft

@jkczyz has more useful tweaks for quantity_max.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>


Header from folded patch 'unsolicited-invoices.patch':

fixup! BOLT 12: offers, fifth draft

Flesh out the handling of unsolicited (e.g. bolt11-style) invoices.
In particular, it has no invreq_payer_id, but still reuses
offer_description, offer_node_id and invreq_chain.

Requirements for responding to an invoice_requests have been reshuffled,
but not changed.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>


Header from folded patch 'not-so-strict-with-disallowed-paths.patch':

fixup! BOLT 12: offers, fifth draft

I think that invoice writers should be able to give us paths with
required features: we don't use them, and obv. reject if there are no
usable paths.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>


Header from folded patch 'fixup-ya.patch':

Allow offer_quantity_max to be 1.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>


Header from folded patch 'invoice_node_id.patch':

BOLT 12: add explicit invoice_node_id.

This is usually equivalent to offer_node_id, but if there's no offer,
it's used by itself.  This simplifies things quite a bit (we always
have a signature in invoices again), and avoids the awkward invoice
code.

In future, we can:
1. Allow delegation (i.e. provide a signature field which shows a delegation
   from the offer_node_id to this invoice_node_id).
2. Maybe allow omitting the offer_node_id, using the first blinded path
   final hop as an implied offer_node_id.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>


Header from folded patch 'more-fixups.patch':

More tweaks from jkczyz.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

iff --git a/12-offer-encoding.md b/12-offer-encoding.md
index 8671024..1459098 100644

Header from folded patch 'remove-definition-of-raw-invoices.patch':

BOLT lightning#12: remove definitions around "raw invoices".

The specification wasn't fully fleshed out for these, so leave them
undefined.  We can always re-add them in future.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Regenerated bolt12/signature-test.json; for some reason jq reordered a
few fields, but it now shows the complete signature.
bolt12/format-string-test.json is now a valid offer.
Offers may contain blinded paths to allow for greater recipient privacy.
However, they come at a cost of increased QR code size as each hop
requires a 33-byte `point` for the `next_node_id`. Allow using
`short_channel_id` instead, which only requires 8 bytes.

Still allow for use of `next_node_id` for cases where the blinded path
may not involve channel counterparties or for long-lived offers, which
may outlive the given channels.
@niftynei niftynei mentioned this pull request Jul 31, 2023
@t-bast
Copy link
Collaborator

t-bast commented Aug 11, 2023

While I definitely understand the appeal, I think that before depending on scid, we should think about what we want to do with scid in the future. They currently directly refer to the channel's funding utxo, which is simple enough. But we have two major features coming that may impact that:

  • splicing: when we splice, should we keep referring to the channel using its initial scid? We probably do, because otherwise we'll invalidate routing maps. Are there any downsides to this approach?
  • private gossip: once we stop advertising the real on-chain utxo for a channel, will we keep some concept of scid or will we get rid of them entirely? How will we avoid conflicts when generating fake scids?

@TheBlueMatt
Copy link
Collaborator

For the next-hop part, SCIDs still work as a way to describe the next hop from a given channel. They don't need to be universally unique in that context, just unique to the source node. I don't see any issue with keeping them around as the "short channel ID" forever.

For the introduction node, indeed, it's more of an issue...

splicing: when we splice, should we keep referring to the channel using its initial scid? We probably do, because otherwise we'll invalidate routing maps. Are there any downsides to this approach?

Absolutely not. Our current gossip approach is to just overwrite the old gossip and let the network think of it as a new channel. Thus, we'll want to use the new SCID and require nodes remember the previous SCID for a few blocks and still allow its use (which we kinda already do).

private gossip: once we stop advertising the real on-chain utxo for a channel, will we keep some concept of scid or will we get rid of them entirely? How will we avoid conflicts when generating fake scids?

Yea this is way more complicated. If we imagine a ZKP world there's no way to get SCIDs. I think this may be far enough away that we don't need to worry about it just yet - if we do simple overcommitment, there's at least always an SCID for one channel that the node has (or, we could make that a requirement), so this proposal doesn't impact that. The proposal does allow for pubkeys still (or it should if not) so at least it'll still work if we move to a ZKP world.

@rustyrussell
Copy link
Collaborator

private gossip: once we stop advertising the real on-chain utxo for a channel, will we keep some concept of scid or will we get rid of them entirely? How will we avoid conflicts when generating fake scids?

Yea this is way more complicated. If we imagine a ZKP world there's no way to get SCIDs. I think this may be far enough away that we don't need to worry about it just yet - if we do simple overcommitment, there's at least always an SCID for one channel that the node has (or, we could make that a requirement), so this proposal doesn't impact that. The proposal does allow for pubkeys still (or it should if not) so at least it'll still work if we move to a ZKP world.

My rough plan for "non-proven channels" was to add shadow ids. That is, if you prove 1111x22x3 as your txout, then you can also own 1111x22+Nx3 where N is the rounded-to-power-of-2 number of txs in block 1111 (thus you cannot merkle prove it). This makes it easy to spot them, and figure out the UTXO whose proof they're sharing, and gives a fair bit of headroom (txnum is 16 bits, usually leaving 10 bits to play with). You only need to remember the number of bits per block.

If we go full zkp, then this breaks down, and scids either become 32-bit local identifiers or are eliminated entirely.

Copy link
Collaborator

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

All looks good except for renumbering...

2. data:
* [`tu64`:`max`]
1. type: 22 (`offer_node_id`)
1. type: 24 (`offer_node_id`)
2. data:
* [`point`:`node_id`]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand the desire for cleanliness, but this breaks everyone's test code. Just make offer_compact_paths number 24 please!

Comment on lines 381 to 388
1. type: 18 (`offer_issuer`)
1. type: 18 (`offer_compact_paths`)
2. data:
* [`...*compact_blinded_path`:`paths`]
1. type: 20 (`offer_issuer`)
2. data:
* [`...*utf8`:`issuer`]
1. type: 20 (`offer_quantity_max`)
1. type: 22 (`offer_quantity_max`)
2. data:
* [`tu64`:`max`]
1. type: 22 (`offer_node_id`)
1. type: 24 (`offer_node_id`)
2. data:
Copy link
Collaborator

Choose a reason for hiding this comment

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

here too

Comment on lines 553 to 582
1. type: 18 (`offer_issuer`)
1. type: 18 (`offer_compact_paths`)
2. data:
* [`...*compact_blinded_path`:`paths`]
1. type: 20 (`offer_issuer`)
2. data:
* [`...*utf8`:`issuer`]
1. type: 20 (`offer_quantity_max`)
1. type: 22 (`offer_quantity_max`)
2. data:
* [`tu64`:`max`]
1. type: 22 (`offer_node_id`)
1. type: 24 (`offer_node_id`)
Copy link
Collaborator

Choose a reason for hiding this comment

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

here too

@jkczyz
Copy link
Author

jkczyz commented Aug 14, 2023

For the introduction node, indeed, it's more of an issue...

Should we consider dropping SCID for the introduction node? It would make the offer format a little cleaner at very least by not having two separate path TLV records... along with removing the implementation complexity that comes along with it.

@rustyrussell
Copy link
Collaborator

Radical idea: a new bolt fundamental type: sciddir_or_pubkey:

  • If first byte is 0 or 1, that's the direction, and it's followed by 8 bytes of scid for 9 byte total.
  • If first byte is 2 or 3, it's a pubkey, for 33 byte total.

There are several places where this could be used, IMHO. And it's backwards compat with existing pubke fields if you always use a pubkey...

Offers may contain blinded paths to allow for greater recipient privacy.
However, they come at a cost of increased QR code size as the
introduction node requires a 33-byte `point`.

Define a new `node_id` fundamental type such that either a point or a
reference to one in a `channel_announcement` can be used. This is
backwards compatible with `point`.

Use this new type for the `blinded_path` subtype's `first_node_id`.
@jkczyz
Copy link
Author

jkczyz commented Aug 14, 2023

Radical idea: a new bolt fundamental type: sciddir_or_pubkey:

  • If first byte is 0 or 1, that's the direction, and it's followed by 8 bytes of scid for 9 byte total.
  • If first byte is 2 or 3, it's a pubkey, for 33 byte total.

There are several places where this could be used, IMHO. And it's backwards compat with existing pubke fields if you always use a pubkey...

Nice! I ended up naming it node_id and defined it in terms of short_channel_id and point. Decided against using "direction" in any way since the first byte simply determines whether node_id_1 or node_id_2 from the channel_announcement is used not which direction something applies to like channel_update. At least in how it is used in blinded_path. Maybe the same doesn't apply to other uses that you are envisioning?

Other alternatives could be:

  • node_id_or_ref
  • point_or_ref

Let me know if you have a preference. One argument against node_id is that it is already used in many field names for point types.

@thomash-acinq
Copy link

I really like the sciddir_or_pubkey.
However I think the main problem here is that channels get opened and closed all the time and we shouldn't rely on an identifier that can disappear at any time. One solution would be to never forget channel ids but that's not very satisfying. Another solution would be for each node to chose one of their channel id and signal that this one should be remembered forever.

@rustyrussell
Copy link
Collaborator

Will fold into #798

@rustyrussell
Copy link
Collaborator

Yeah, "node_id" is counter-intuitive (internally, we have a node_id type already, it's a SECP1-encoded u8[33]). sciddir_or_pubkey is clunky, but works, so I'll switch to that...

@rustyrussell
Copy link
Collaborator

Hmm, since #798 has been rebased, there was a significant conflict with the first patch. I think I got it right (though I insisted on exaclty one of scid or nodeid).

@t-bast
Copy link
Collaborator

t-bast commented Oct 2, 2023

@jkczyz since this has been integrated into #798, I believe we can close this PR?

@jkczyz jkczyz closed this Oct 3, 2023
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

5 participants