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

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.

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
Copy link
Collaborator

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
Copy link
Contributor Author

@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
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
Copy link
Contributor Author

@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.

@t-bast t-bast added the Meeting Discussion Raise at next meeting label Jun 14, 2019
07-routing-gossip.md Outdated Show resolved Hide resolved
09-features.md Outdated

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

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't it below at line 45? Rationale

@araspitzu araspitzu force-pushed the large_channel_support branch from 8c12568 to a41e33e Compare July 3, 2019 09:30
@araspitzu
Copy link
Contributor Author

Summary of the latest changes:

@niftynei niftynei removed the Meeting Discussion Raise at next meeting label Jul 22, 2019
@t-bast t-bast added the Meeting Discussion Raise at next meeting label Aug 5, 2019
rustyrussell and others added 6 commits September 17, 2019 14:51
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>
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
Copy link
Contributor Author

Summary of the changes up to 8af0535:

09-features.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@t-bast t-bast left a comment

Choose a reason for hiding this comment

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

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
Copy link
Contributor Author

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

Copy link
Collaborator

@t-bast t-bast left a comment

Choose a reason for hiding this comment

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

ACK d5a8978

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

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

Copy link
Collaborator

@t-bast t-bast left a comment

Choose a reason for hiding this comment

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

ACK 660dd2e

@t-bast
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Meeting Discussion Raise at next meeting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants