ElementsProject / lightning

Core Lightning — Lightning Network implementation focusing on spec compliance and performance
Other
2.84k stars 901 forks source link

listnodes returns giant feature bits for some nodes #3666

Closed fiatjaf closed 4 years ago

fiatjaf commented 4 years ago

Issue and Steps to Reproduce

This one, for example, is it because the node is advertising a very unusual featureset or is it some bug in the parsing code?

~> lightning-cli listnodes 02a447fd201226f0bf6421c356f9d2117b0fb05ccc0858dd2b20589b9edb488f67
{
   "nodes": [
      {
         "nodeid": "02a447fd201226f0bf6421c356f9d2117b0fb05ccc0858dd2b20589b9edb488f67",
         "alias": "France HUB 🇫🇷",
         "color": "0d00c7",
         "last_timestamp": 1587714056,
         "features": "8000000000000000000002aaa2",
         "addresses": [
            ...
         ]
      }
   ]
}

Apparently it happens only with these four nodes:

026fe566f268b8e88512cfb8ffda1426416dde5f8bbf0badf43007165be0f2bf76
024b9a1fa8e006f1e3937f65f66c408e6da8e1ca728ea43222a7381df1cc449605
02a1d2856be336a58af08989aea0d8c41e072ccc392c46f8ce0e6e069f002035f3
02a447fd201226f0bf6421c356f9d2117b0fb05ccc0858dd2b20589b9edb488f67
cdecker commented 4 years ago

This is normal and happens when a node announces custom or experimental features, which uses higher position bits and this expands the featurebits vector to those dimensions.

In this case it seems like the nodes are signalling feature 103 which I could not find documentation about anywhere.

darosior commented 4 years ago

This happens to be a node I manage, and I run current rc1 without fancy feature-registering plugins (wild guess: maybe keysend ?)

EDIT: They are all running C-lightning and surely rc1 (mine and Rusty's for sure the others don't advertise it), so this must be something we introduced.

fiatjaf commented 4 years ago

Interesting, I didn't know plugins could change the advertised featureset of the node. Saw it now.

darosior commented 4 years ago

@fiatjaf why did you close this ? I don't think we should advertise feature bit 103 (and not 55, the keysend one)

fiatjaf commented 4 years ago

I don't know, I thought it was something a rogue plugin could do and it ended there. I also thought I was running rc1 but it seems that for some reason I'm not. Let me see what happens when I upgrade.

fiatjaf commented 4 years ago

Yeah, I actually was on master, but for some reason getinfo keeps saying I'm on 0.8.0: v0.8.0-504-g10f47b4. And I have nothing like that on my features string.

darosior commented 4 years ago

How did you check it ? listnodes <yourid> ?

cdecker commented 4 years ago

Interesting, I dismissed this initially because I couldn't find the number 103 in the codebase, but it seems I was misusing the feature_bit API. Digging now.

cdecker commented 4 years ago

Ok, after checking further I found a real bug in the way the keysend plugin (based on libplugin) registers its features. Turns out I was adding a dict-key called featurebits in libplugin. This matches what the docs said, but not what I implemented.

lightningd looks for a key called featurebits and, not finding it, it was just discarding whatever libplugin was telling it.

I fixed that in #3673

After further inspection I also finally found the origin of feature bit 103 being set:

https://github.com/ElementsProject/lightning/blob/10f47b41fa3192638442ef04d816380950cc32c9/common/features.h#L109

This is the new blinded path messaging framework implemented by @rustyrussell in #3623. These features are defined in their mandatory form (102) and then added to the featurebit vector depending on the copy-mode, which for this one is to make it options (103). So the mystery is finally solved, and we even fixed something along the way :-)