-
Notifications
You must be signed in to change notification settings - Fork 493
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
Public key-based routing (Feature **NEEDS NUMBER***) #814
base: master
Are you sure you want to change the base?
Conversation
Why not work on reviewing #765 instead, which fixes that and adds other goodies? |
I like small steps. |
But the risk is cluttering the spec with things that quickly become obsolete...IMHO the spec is exactly where we want to spend time reviewing, get frustrated sometimes because things are moving slow (already 6 months since route blinding has been proposed), but end up with a solution that is great for everyone. I think we don't want half-measures here, and leaking the |
It depends on what problem you want to address. I am mainly concerned with safety. Don't tell anyone how many bitcoins you have. This PR fixes just that. I totally agree that it would be nice to do more, but a larger change also carries more risk of getting pushed back. Nevertheless thanks for sharing your view on the scope. I'd be interested to hear other people's opinions. Perhaps it turns out that the only purpose of this PR is to underline that we want to do full obfuscation right from the start. |
Solution here IMO is just to go with distinct documents in a more BIP-like format. We should really put an end to all the "conditional" statements in the spec as is. You can describe your proposal, get feedback, and ppl can implement it or not. Everyone has different priorities and resources they can allocate to changes they care about in the end. People only really care about changes that directly impact them, or that they have some sort of higher level need for, and in the end we have quite a bit of ways to extend the protocol as is. |
As an example, this can be a very small stand alone document that describes the motivation and what changes it requires (just a new TLV). That can then be linked from the routing section as additional features implementors may want to implement. This is nice also since it starts to separate the "base" protocol from other enhancements. The feature bit system lets clients know what they should and shouldn't include. In the end, things can move a lot more quickly, with people only opining on what they actually care about. |
I agree, that's already what has been done for route blinding and trampoline routing (at least on the format, and put in a |
I can convert it to a separate proposal, but first wanted to see if there is enthusiasm for this change at all. I think that for just assessing that, a diff may even be better because it quickly shows where the change impact is. |
A poor man's version of pubkey routing could be to use the truncated pubkey as the channel id. Routing nodes would first interpret the channel id as a real id and if that fails try matching with the channel id as a truncated pubkey. Not my preference, but also no tlv changes needed. |
True, maybe we just need a 2 impl (as if one impl is doing something own their own, they doesn't necessarily need a spec?) sign off? Alternatively, if only a single impl is doing something, still likely nice to have good documentation/rationale for it in case others want to implement them as well. For that we could just fallback to like a "hard reject, do no harm" type rule. If we want to refer to the "extensions" numerically, we could do what ETH does and name them after the fact using the PR number itself. |
That sounds good to me:
|
Totally agree, this was the main reason I started pushing for separate proposal documents, that walk through the rationale and contain all the relevant pieces in one place in order to catch up with a proposal, and for the living spec to reference for further details. Ulimately, the spec should be a document that reads like an implementation, without all the discussion fluff, and the individual proposals should be self-contained documents shedding light on individual aspects of that implementation. |
@@ -262,6 +262,9 @@ This is a more flexible format, which avoids the redundant `short_channel_id` fi | |||
2. data: | |||
* [`32*byte`:`payment_secret`] | |||
* [`tu64`:`total_msat`] | |||
1. type: 10 (`pub_key`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about outgoing_node_id
?
I already have added this field in the trampoline proposal (I believe also with TLV type 10) so I'm interested in getting it spec-ed anyway :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that it is a good idea to have two unapproved prs reference each other. Suppose this PR would be approved first, then renaming the field is easy enough and won't change the wire protocol.
09-features.md
Outdated
@@ -39,6 +39,7 @@ The Context column decodes as follows: | |||
| 16/17 | `basic_mpp` | Node can receive basic multi-part payments | IN9 | `payment_secret` | [BOLT #4][bolt04-mpp] | | |||
| 18/19 | `option_support_large_channel` | Can create large channels | IN | | [BOLT #2](02-peer-protocol.md#the-open_channel-message) | | |||
| 20/21 | `option_anchor_outputs` | Anchor outputs | IN | `option_static_remotekey` | [BOLT #3](03-transactions.md) | | |||
| 22/23 | `option_pubkey_routing` | Public key routing | IN | `var_onion_optin` | [BOLT #4](04-onion-routing.md#tlv-payload-format) | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the flags be IN9
?
It feels like you need to set it to required on your invoice when you're using it, otherwise senders will not correctly include the outgoing_node_id
field in the onion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is that senders recognize the all-zeroes channel id and use this as a cue to switch to pubkey routing. But for older senders this will just lead to failures that an invoice feature bit can prevent. Added the 9.
104e7f4
to
7a73e7d
Compare
Feature number clash with #824 Can we move this to 26/27 as I've suggested in the title? |
7a73e7d
to
e27f165
Compare
Feature bits updated |
This now clashes with #672. I'd suggest to move to 30/31? |
Maybe I missed something, but what's the point of mentioning non-final feature bits in the title @rustyrussell ? |
It's there so we can detect these clashes early on, before people start implementing and signalling online. |
Yeah, I just noticed that this clashes with the merged option for anysegwit. Please renumber if this is still a desired change? |
In today's Lightning Network, routes are described as a series of short channel ids. Routing nodes use these short channel ids to determine to which of its peers an htlc needs to be forwarded.
The advantage of short channel ids is that they are short - just 8 bytes.
A great downside is that a short channel id corresponds directly to a channel point on-chain. A channel point reveals the size of the channel and is thereby an indication of the amount of funds that are kept by the channel parties in their node hot wallets. Large hot wallets are an attractive target to hackers.
For public channels, this isn't an issue. To advertise a public channel, the channel point needs to be revealed anyway so that other node are able to verify the existence of the channel.
For private channels it is different. Private channels are only disclosed in invoice route hints and don't need to be verified. The invoice is created and signed by the receiver and there is no reason to include a non-existing channel in there. Ideally the invoice does not contain channel points. Otherwise someone can request an invoice (or multiple invoices) from a service that accepts Lightning payments and inspect the route hints to gauge the service's node capacity.
This PR proposes a new routing mode based on full public keys instead of short channel ids. When nodes signal
option_pubkey_routing
, they'll also accept a public key in the tlv onion payload to control the outgoing direction of the htlc.With this feature, receivers can strip the channel points of private channels from invoices for peers that support
option_pubkey_routing
. Senders need to construct pubkey tlv payloads for the final private leg towards the destination. This increases the payload size with an extra 25 bytes. But as this is only required for the last (or last few) hops, the reduction of the maximum route length isn't very significant.Optionally the invoice format can be changed so that short channel ids can be omitted completely from route hints, saving 8 bytes per route hint.
Overall a relatively simple change that makes Lightning safer to use for network endpoints in particular.