BOLT-09: move static remote to global feature bit

[Roasbeef]

In this commit, we move the new static remote feature bit from a local
feature bit, to a global feature bit. This should be a global bit as
peers will want to seek out others peers based on if they support the
new safety feature or not. Having it as a local feature bit means they
need to connect out, and complete the handshake before finally being
able to determine if a peer supports this new safety feature or not.

[t-bast]

Concept ACK, but why not finalize #666 instead?
It renders this distinction moot as #666 broadcasts local + global in node_announcement.

[Roasbeef]

@t-bast #666 is a bigger overhaul, it's starting to creep into updating the existing hop hints to have feature bits for each channel/node. I don't think we'll have enough time to test+implement that for our upcoming release without delaying it by several weeks potentially. This is a simple move to how things should've been initially IMO.

[t-bast]

it's starting to creep into updating the existing hop hints to have feature bits for each channel/node.

Honestly I don't think this is necessary for #666. I think #666 is ready as-is and what you propose is a further enhancement for a follow-up PR.

However I do agree that it's more testing than just moving the static_remote_key bits to global features, which is indeed where it should have been, so we can merge this PR whenever you want.

[rustyrussell]

However I do agree that it's more testing than just moving the static_remote_key bits to global features, which is indeed where it should have been, so we can merge this PR whenever you want.

Huh? No, it's definitely not a global feature, as it only affects direct peers.

However, like every local feature, it's interesting to broadcast it, so we should advertize it in node_announcement (which is what c-lightning master now does, along with every other local feature).

So I would say: definitely advertize it in node_announcement for your coming release. Whether you treat it as a local or global feature bit now only affects the init msg. The spec will catch up, and it's definitely legal since we decided feature bits will be distinct between global/local already?

[Roasbeef]

It's a global feature insofar as it may affect _which_ peers you open channels or connect with. Arguably the same should've been done for DLP.

…

On Mon, Oct 7, 2019, 7:20 PM Rusty Russell ***@***.***> wrote: However I do agree that it's more testing than just moving the static_remote_key bits to global features, which is indeed where it should have been, so we can merge this PR whenever you want. Huh? No, it's definitely not a global feature, as it only affects direct peers. However, like every local feature, it's interesting to broadcast it, so we should advertize it in node_announcement (which is what c-lightning master now does, along with every other local feature). So I would say: definitely advertize it in node_announcement for your coming release. Whether you treat it as a local or global feature bit now only affects the init msg. The spec will catch up, and it's definitely legal since we decided feature bits will be distinct between global/local already? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#680>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAHTWLUO3EGQTJBEFCFZKG3QNPU7RANCNFSM4I42DPTQ> .

[araspitzu]

To me it looks like a local feature because it only affects the two peers in the channel, a peer being selective when choosing the remote side doesn't justify having this feature global since it's not really used for any purpose than that (i.e routing), anyway #666 trumps these discussion since it allows having a local feature broadcast globally.

It's a global feature insofar as it may affect which peers you open channels or connect with. Arguably the same should've been done for DLP.
…
On Mon, Oct 7, 2019, 7:20 PM Rusty Russell @.***> wrote: However I do agree that it's more testing than just moving the static_remote_key bits to global features, which is indeed where it should have been, so we can merge this PR whenever you want. Huh? No, it's definitely not a global feature, as it only affects direct peers. However, like every local feature, it's interesting to broadcast it, so we should advertize it in node_announcement (which is what c-lightning master now does, along with every other local feature). So I would say: definitely advertize it in node_announcement for your coming release. Whether you treat it as a local or global feature bit now only affects the init msg. The spec will catch up, and it's definitely legal since we decided feature bits will be distinct between global/local already? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#680?email_source=notifications&email_token=AAHTWLXIV3VOHC5H27ZP2LTQNPU7RA5CNFSM4I42DPT2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEASM7RY#issuecomment-539283399>, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHTWLUO3EGQTJBEFCFZKG3QNPU7RANCNFSM4I42DPTQ .

[rustyrussell]

No, it was never meant to be a global bit. It doesn't make any more sense than any existing bit. Just advertize them both in node_announcement.

[Roasbeef]

This is about the init message, not the node announcement. As is, there hasn't yet been any grand unification. Looking at the eclair PR, they've already switched over and are on board (based on t-bast's first comment) with this switch.

…

On Tue, Oct 8, 2019, 5:05 PM Rusty Russell ***@***.***> wrote: No, it was never meant to be a global bit. It doesn't make any more sense than any existing bit. Just advertize them both in node_announcement. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#680>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAHTWLWE2UPDPKVBSQCGZI3QNUN4JANCNFSM4I42DPTQ> .

[t-bast]

Honestly our thought about this is that we'd like to make progress on #666 soon, which deprecates arguing about whether this is a global or local feature ;)
I think this feature (and a few others) shows that what's important is how you communicate feature bits to others and being able to communicate what we currently call local features in node announcements (and potentially other messages as well).
So we're ok with either of those since this is very-soon-to-be-deprecated, if that helps lnd's release be smoother we don't mind putting this in the global field.

[Roasbeef]

As I said above #666 has a much larger scope, and there're still questions of proper backwards compatibility. IMO it's too aggressive w.r.t assumptions of the the level of synchronization that currently exists amongst the various implementations when rolling our new updates. It effectively removes the global field from the init message, thereby ignoring the possibility of other implementations in the wild that may be using that space to gate experimental new features.

This just moves 2 bits around. I'm not sure how soon this "soon" will really be given many of us have travel plans upcoming over the next few weeks as well.

[Roasbeef]

I spoked with Rusty offline, and CL is planning to move to just advertising all their bits in both fields. This works as we don't have any overlap between local/global bits right now. It effectively implements a superset of what's in this PR.

AFAICT, the eclair PR still looks at the global init features as well. So it appears we're all aligned on this front now (#666 withstanding).

[t-bast]

Eclair can align on whatever is chosen here, we plan on doing the same as CL right after the static_remote_key PR is integrated in master so we don't really mind.

[Roasbeef]

Now a subset of #666.