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.

Sign up for GitHub

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

Jump to bottom

HTLC blinded route support (EXPERIMENTAL) #3623

Merged
merged 10 commits into from
Apr 14, 2020
Merged

HTLC blinded route support (EXPERIMENTAL) #3623

merged 10 commits into from
Apr 14, 2020

Conversation

rustyrussell
Copy link
Contributor

@rustyrussell rustyrussell commented Apr 5, 2020 •
edited
Loading

This is an implementation of https://github.com/lightningnetwork/lightning-rfc/blob/route-blinding/proposals/route-blinding.md (only with --enable-experimental-features).

In this method, the vendor provides a blinded path similar to route hints, but the node ids are blinded, and instead of short_channel_ids for each hop, it provides an encrypted blob. This can be used for (finally, genuinely!) private channels as well as fully private vendor paths. Every node on the blinded path needs to be upgraded, however.

This is the minimum implementation to allow interop testing (e.g. with @t-bast when he gets Eclair done). There are five things remaining to be done:

  1. We need to advertize a feature bit (simple to fix)
  2. We currently use a dummy short_channel_id field, which is unnecessary in blinded paths (a bit more complex, since our internals assume a short_channel_id field in every hop).
  3. We don't propagate errors properly (we need to decide how we'll do this).
  4. We haven't yet defined a bolt11 encoding for blinded paths.
  5. We don't validate padded entries in the blinding (we could add them, but we need to check they're valid, otherwise the payer can corrupt them and discover if it was a dummy entry).

It's probably premature to add those until we have a solid spec (esp. how to encode blinded routes in bolt11; presumably this will involve emojis!).

Changelog-None: Experimental only

@rustyrussell rustyrussell added spec Optech Make Me Famous! Look! Look! Look! COOL NEW FEATURE! labels Apr 5, 2020
@rustyrussell rustyrussell added this to the 0.8.2 milestone Apr 5, 2020
@rustyrussell rustyrussell requested a review from cdecker as a code owner April 5, 2020 01:59
@rustyrussell rustyrussell force-pushed the guilt/onion-messages branch 2 times, most recently from 835a9d5 to 8c960cd Compare April 5, 2020 23:40
@t-bast
Copy link

t-bast commented Apr 6, 2020 •
edited
Loading

I did a quick pass on this, nothing looks too weird but my C skills are quite rusty (bad pun intended).
I'll get a prototype working on eclair this week.

On the proposal itself, there are three main areas on which I'd like more eyes on:

  • the crypto itself, and whether we're missing an abstraction (it's really a kind of reverse sphinx)
  • whether we can live with some update_add_htlc that have a different length than other update_add_htlc (which kind of gives away the blinded path if you're watching the network)
  • error management: how do we ensure errors don't leak the blinded path? Obfuscation + delayed responses may do the trick, but is it enough (and is it necessary)?

I turned my blinded path into a draft PR so that others can start having a look at it: lightning/bolts#765

@t-bast
Copy link

t-bast commented Apr 7, 2020

I have a very rough prototype for eclair here: https://github.com/ACINQ/eclair/tree/route-blinding
It should be enough for some interop testing, I can generate a shared test vector if you want.
It made me realize that there are a few places where the spec proposal is vague, and implementation could slightly differ.

niftynei
niftynei reviewed Apr 9, 2020
View reviewed changes
Copy link
Collaborator

@niftynei niftynei left a comment

Choose a reason for hiding this comment

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

commit 8c960cd is showing as empty/contentless for me?

@t-bast
Copy link

t-bast commented Apr 10, 2020

commit 8c960cd is showing as empty/contentless for me?

Because the best code is no code at all

@rustyrussell
features: add EXPERIMENTAL option_onion_messages from draft.
8a2d48a 
Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell
EXPERIMENTAL: protocol TLV field for update_add_htlc to support blind…
6969551 
…ing.

All NULL for now.

Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell
EXPERIMENTAL: onion TLV field for enctlv for blinding.
09448cc 
Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell
EXPERIMENTAL: new error code for use when HTLC is blinded.
0d0ff3e 
Everyone should return this for any HTLC error when they're handed a blinding
alongside the HTLC.

Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell
common/onion: EXPERIMENTAL handling of enctlv field to override next_…
3473fad 
…short_channel_id.

This requires us to call ecdh() in the corner case where the blinding seed
is in the TLV itself (which is the case for the start of a blinded route).

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

commit 8c960cd is showing as empty/contentless for me?

Huh... not sure how that happened. That's why backups FTW, one moment...

@rustyrussell
channeld: support HTLCs with blinding (EXPERIMENTAL_FEATURES)
d20321c 
Note that it's channeld which calculates the shared secret, too.  This
minimizes the work that lightningd has to do, at cost of passing this
through.

We also don't yet save the blinding field(s) to the database.

Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell
lightningd: add blinding and enctlv field to struct route_hop.
b06af1d 
This will be used when we want to specify these in a route.  But for now, they
only alter gossipd, which always sets them to NULL.

Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell
common/onion: add blinding and enctlv encoding.
a7bf86d 
Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell
devtools/blindedpath: implement scid support.
fe852f0 
This is what we expect in an enctlv used for htlcs.

Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell
lightningd: make sendpay support blinding and enctlv fields (EXPE…
74c1f9e 
…RIMENTAL)

This is what actually lets us pay blinded invoices.

Unfortunately, our internal logic assumes every hop in a path has a
next `short_channel_id`, so we have to use a dummy.  This is
sufficient for testing, however.

Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell rustyrussell force-pushed the guilt/onion-messages branch from 8c960cd to 74c1f9e Compare April 11, 2020 03:50
@niftynei
Copy link
Collaborator

ACK 74c1f9e

@niftynei
Copy link
Collaborator

experimental means no changelog required.

@rustyrussell rustyrussell merged commit 1d29228 into ElementsProject:master Apr 14, 2020
@cdecker cdecker mentioned this pull request Apr 27, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@niftynei niftynei niftynei left review comments

@cdecker cdecker Awaiting requested review from cdecker

Assignees
No one assigned
Labels
Optech Make Me Famous! Look! Look! Look! COOL NEW FEATURE! spec
Projects
None yet
Milestone
0.8.2
Development

Successfully merging this pull request may close these issues.
3 participants
@rustyrussell @t-bast @niftynei