Skip to content

BOLT 9: Make it clear that 'channel_public' apply to all channels#113

Merged
rustyrussell merged 1 commit into
lightning:masterfrom
pm47:channels_public
Feb 21, 2017
Merged

BOLT 9: Make it clear that 'channel_public' apply to all channels#113
rustyrussell merged 1 commit into
lightning:masterfrom
pm47:channels_public

Conversation

@pm47
Copy link
Copy Markdown
Collaborator

@pm47 pm47 commented Feb 16, 2017

This follows #109 and Christian's comment in #97.

I have also renamed channel_public to channels_public.

@Roasbeef
Copy link
Copy Markdown
Collaborator

Hmm, why not make this more granular and instead allow nodes to signal their willingness to advertise per-channel?

This route gives nodes more flexibility. With this, nodes don't need to rear down and reestablish a new connection in the case that they want an additional link to another node that has a network visibility policy opposite to that of any prior channels they have open with that node.

@cdecker
Copy link
Copy Markdown
Collaborator

cdecker commented Feb 17, 2017

While it surely adds flexibility I don't think we want to add yet another flags field to another message. We're not even sure if people will multiplex channels at all, so adding this here is a bit of an overkill, especially since it can be amended later (featureflag on the connection, signalling that we append a features field to channel_open and channel_accept).

Comment thread 09-features.md Outdated
| Bits | Name |Description | Link |
|------|------------------|------------------------------------------------|---------------------------------------------------------------------|
| 0/1 | `channel_public` | The sending node wishes to announce the channel | [BOLT #7](07-routing-gossip.md#the-announcement_signatures-message) |
| 0/1 | `channels_public` | The sending node wishes not to announce channels | [BOLT #7](07-routing-gossip.md#the-announcement_signatures-message) |
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You inverted the bit? I'm confused....

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

That's strange indeed, it also clashes with the name for the bits.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Hmmm, I don't know why I did that, how embarrassing! Fixed...

@pm47
Copy link
Copy Markdown
Collaborator Author

pm47 commented Feb 20, 2017

@Roasbeef: the purpose of this PR was just to fix an inconsistency in the spec. I would also prefer to be able to set this parameter at the channel level.

FWIW I see another potential issue with setting it at the connection level: in case of a disconnection, one or both nodes may change their parameter before reconnecting, and it can happen before we finished the channel setup and did/didn't publish the announcements. Which means we probably need to store this parameter at the channel level, and also that we can have both public and private channels in the same connection independently of the current value of the parameter.

@rustyrussell rustyrussell merged commit 6e8fe9d into lightning:master Feb 21, 2017
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.

4 participants