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

Single-option large channel proposal #596

Merged
merged 19 commits into from Feb 18, 2020

Conversation

@araspitzu
Copy link
Contributor

araspitzu commented Apr 2, 2019

This PR allows peers to lift the channel size limit with a single option option_support_large_channel, it subsumes the previous PR #590. The option means that the offering peer supports opening large channels, a peer willing to send a large htlc should refer to channel_update.htlc_maximum_msat to check if a certain channel can route that.

rustyrussell added 5 commits May 6, 2019
Rename `globalfeatures` to `channelfeatures` and `localfeatures`
to `nodefeatures`.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
You can still gossip.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If we really want a new gossip message which old nodes will ignore,
we'll use a new type, so having it discard unknown features is
overzealous.

Each feature can itself specify how it's advertized here: an
key-exchange-instead-of-hash-preimage feature would need to advertize
as even (you need to understand it to use it), for example, but a
wumbo feature would advertize as odd.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This lets you find out what nodes support what node features, rather
than connecting and probing.

Like channel_announcement, we won't use feature bits for incompatible
changes; we'll use a separate type.  So don't discard messages with
unknown ones.

Similarly, you can try to connect to a node with unknown bits; you
might fail, but that's OK.  Either it was an unknown node feature, and
you'll find out from their init msg, or it's a channel feature and you
won't be able to open a channel.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
09-features.md Outdated Show resolved Hide resolved
@rustyrussell

This comment has been minimized.

Copy link
Collaborator

rustyrussell commented Jun 10, 2019

Technically, this is a local/node feature since it only effects direct peers.

And #571 supports advertizing those features via node_announcement, see 14ced3b

@araspitzu

This comment has been minimized.

Copy link
Contributor Author

araspitzu commented Jun 11, 2019

@rustyrussell right! I took the wrong bits from the temporary assignment in #571, this was meant to use the nodefeature bits as in what previously was called option_i_wumbo_you_wumbo with bits 14/15, I will update the PR.

@t-bast

This comment has been minimized.

Copy link
Collaborator

t-bast commented Jun 11, 2019

After re-reading through #571, I agree with @rustyrussell: this is a node feature.

In #605 there are both node and channel feature flags for this. I think the option_wumborama channel flags aren't useful, should we remove them? Or can you explain why you think they are useful? In my opinion just looking at the channel capacity and htlc_maximum_msat give you all the information you need for routing.

@araspitzu

This comment has been minimized.

Copy link
Contributor Author

araspitzu commented Jun 11, 2019

@t-bast indeed this PR does not propose to use option_wumborama on the channel flags but uses a single-option and it should go in the node features (once we unify feature bits). As per description the option means that the offering peer supports opening large channels, a peer willing to send a large htlc should refer to channel_update.htlc_maximum_msat to check if a certain channel can route that.

07-routing-gossip.md Outdated Show resolved Hide resolved

| Bits | Name |Description | Link |
|------|------------------|------------------------------------------------|---------------------------------------------------------------------|
| 14/15 | `option_support_large_channel` | Can create large channels | [Rationale](#Rationale) |

This comment has been minimized.

Copy link
@rustyrussell

rustyrussell Jun 24, 2019

Collaborator

I think that link is broken, since Rationale is in a separate BOLT?

This comment has been minimized.

Copy link
@araspitzu

araspitzu Jun 25, 2019

Author Contributor

Isn't it below at line 45? Rationale

@araspitzu araspitzu force-pushed the araspitzu:large_channel_support branch from 8c12568 to a41e33e Jul 3, 2019
@araspitzu

This comment has been minimized.

Copy link
Contributor Author

araspitzu commented Jul 3, 2019

Summary of the latest changes:

  • Rebased on #571
  • Updated feature bits to 8/9
  • Moved rationale to BOLT-2 replacing old section about the current limit
@t-bast t-bast moved this from Scheduled to Accepted in Specification Meeting Agenda Jul 8, 2019
@t-bast t-bast moved this from Accepted to Scheduled in Specification Meeting Agenda Aug 19, 2019
rustyrussell and others added 6 commits Sep 17, 2019
We simply specify, in each case, where they will appear ("Context").
We replace the `globalfeatures` field in init with a zero.

Note also: we REQUIRE minimal `features` field in
channel_announcement, since otherwise both sides of channel will not
agree and not be able to create their signatures!

Consider these theoretical future features:

`opt_dlog_chan`: a new channel type which uses a new discrete log HTLC
type, but can't support traditional HTLC:

* `init`: presents as odd (optional) or even (if traditional channels
  not supported)
* `node_announcement`: the same as above, so you can seek suitable peers.
* `channel_announcement`: presents as even (compulsory), since users need
  to use the new HTLCs.

`opt_wumbochan`: a node which allows channels > 2^24 satoshis:

* `init`: presents as odd (optional), or maybe even (if you only want
  giant channels)
* `node_announcement`: the same as above, so you can seek suitable peers.
* `channel_announcement`: not present, since size of channel indicates
  capacity.

`opt_wumbohtlc`: a channel which allows HTLCs > 2^32 millisatoshis:

* `init`: presents as odd (optional), or even (compulsory)
* `node_announcement`: the same as above, so you can seek suitable peers.
* `channel_announcement`: odd (optional) since you can use the channel
  without understanding what this option means.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Restore gflen field, as (unreleased) implementations are using it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The feature fields refer to the properties of the channel/node, not the
message itself, so we can still propagate them (and should, to avoid
splitting the network).

If we want to make an incompatible announcement message, we'll use a
different type, or insert an even TLV type.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fix-up!
Co-Authored-By: Bastien Teinturier <31281497+t-bast@users.noreply.github.com>
…hannel_support

# Conflicts:
#	01-messaging.md
#	02-peer-protocol.md
#	04-onion-routing.md
#	07-routing-gossip.md
#	09-features.md
@araspitzu

This comment has been minimized.

Copy link
Contributor Author

araspitzu commented Oct 3, 2019

Summary of the changes up to 8af0535:

  • Uses flat features from #666
  • Uses feature bits 14/15 (they're mostly placeholders until actual bits are assigned)
09-features.md Outdated Show resolved Hide resolved
@t-bast
t-bast approved these changes Oct 4, 2019
Copy link
Collaborator

t-bast left a comment

ACK! We can hopefully merge that around the same time we merge #666

# Conflicts:
#	01-messaging.md
#	02-peer-protocol.md
#	09-features.md
@araspitzu araspitzu force-pushed the araspitzu:large_channel_support branch from d304484 to 6f08b6e Nov 26, 2019
@t-bast t-bast mentioned this pull request Jan 21, 2020
11 of 17 tasks complete
# Conflicts:
#	09-features.md
@araspitzu

This comment has been minimized.

Copy link
Contributor Author

araspitzu commented Jan 27, 2020

Commit d5a8978 resolves conflicts with master and updates the feature bits to 48/49, those requested in the waiting room #605

@t-bast
t-bast approved these changes Jan 29, 2020
Copy link
Collaborator

t-bast left a comment

ACK d5a8978

09-features.md Outdated Show resolved Hide resolved
araspitzu and others added 2 commits Feb 3, 2020
Add missing column

Co-Authored-By: Bastien Teinturier <31281497+t-bast@users.noreply.github.com>
@araspitzu

This comment has been minimized.

Copy link
Contributor Author

araspitzu commented Feb 4, 2020

As discussed on the last spec meeting I've updated the feature bits to 18/19 in commit 660dd2e cc: @cdecker

@t-bast
t-bast approved these changes Feb 4, 2020
Copy link
Collaborator

t-bast left a comment

ACK 660dd2e

@t-bast

This comment has been minimized.

Copy link
Collaborator

t-bast commented Feb 18, 2020

Merging as agreed during the meeting: http://www.erisian.com.au/meetbot/lightning-dev/2020/lightning-dev.2020-02-17-19.06.html

A follow-up PR with an advisory for confirmation scaling depending on funding amount will be submitted shortly.

@t-bast t-bast merged commit 3847935 into lightningnetwork:master Feb 18, 2020
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.