-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
possible issue with explicit chan type negotiation compatibility with eclair nodes #5890
Comments
This was changed previous to the referenced commit: #5669 (comment) |
Yep, should be documented in the release notes. The issue is that in the prior version, the "default" type (0), points to the legacy commitment format, that we actually want to fully deprecate in future versions. This meant that with the new channel type funding option, if a user didn't specify a value, then they'd end up using the most insecure channel type. The change moves "unknown" to zero, which lets us default that to the "most secure" channel type. As an example, this currently defaults to anchors, and later will be w/e initial taproot variants lands, etc, etc. Note that this message wasn't used in any requests until now, it instead changed a field in a response, that also wasn't used anywhere else. |
Wasn't it an option to create a new enum just for the channel funding option? I think the message was used in the |
One option would be to keep the current enum the same but deprecate it, and create a new commitment type enum for future types |
The old one wasn't used for any input, so we decided to just make a new version that's more fool proof. We'll follow the uptake during the rc phase, and are open to alternative PRs that affect existing applications less. |
I don't understand why you would make a breaking change if not absolutely necessary. It also isn't that a change in the response can't have serious consequences. Definitely wouldn't await the rc and risk not enough people reporting it. |
Please make sure that this doesn't break connectivity to non-lnd nodes that don't support anchor channels. In my case: I run lnd 0.14 (master) and have troubles opening a channel to ACINQ (despite already having two open channels). lnd complains about "unexpected channel type". My invocation attempt:
|
This is unrelated, that's due to a prior version using the wrong feature bit for new explicit channel negotiation. Are you still getting that error on the current master @C-Otto, if so then we need to investigate that further. See #5874
The change is more future proof, as it means people don't default to known insecure or replaced channels. Before there was no way to have a "safe" default. The field wasn't used in any requests up until now, so we felt the impact wasn't as large, as say breaking an API by removing/modifying a field used as a request. As mentioned above, if you want to propose alternatives, feel free to open a PR. |
@Roasbeef yes, I still get the error shown above when I try to open a channel to SilentBob (02e9046555a9665145b0dbd7f135744598418df7d61d3660659641886ef1274844), which runs Eclair as far as I know. This is with bos 11.8.2 and lnd 0.14.0-beta-rc3.
|
I get this issue when SilentBob tries to open a channel to my node:
|
Get the same error as C-Otto also with 0.14.0-beta-rc4:
|
Did the update to lnd 0.14.0-beta. The problem still occurs. |
@C-Otto sounds like an older eclair node? That's returned if you're setting a an explicit channel type, but the other party doesn't actually have the feature bit set. |
@ABAPRules IIRC that node is an eclair node, you'll see that error if we didn't trigger explicit channel funding, but they sent a message that tries to utilize it. |
Looking into the compat issue w/ eclair, seems like a difference w.r.t when the bit is set and what the other side expects if it's signalled as optional. |
enum CommitmentType
?
Operator of SilentBob here. My eclair version is the very latest: 0.6.2 |
Ok I think I have a fix here, will fast-follow with a minor release. |
I had the same issue on v0.14.0 when opening channel to a node running c-lightning v0.10.2. |
@vertiond do you have logs of the error you get w/ that version of CL? |
|
This is pretty bad and needs fixing ASAP! LND is now incompatible with eclair and c-lightning and neither side can open channels to the other! As more and more people upgrade their LND nodes to 0.14.0 the network gets more and more partitioned! Imho the latest release should be pulled immediately until it's fixed and rereleased with a high alert note to get people to upgrade their broken v.0.14.0 versions! |
So in the end, lnd's validation of a new feature was too strict, this was cleared up in this to-be merged spec PR: lightning/bolts#906. Not that this only affected the newest versions of CL+eclair that shipped with this new feature. We've prepared #6026 which bundles this fix along with others, that will be released as part of a quick-follow 0.14.1 |
In commit 235efc0,
lightning.proto
defines:But in master cac8da8 this changed to:
The text was updated successfully, but these errors were encountered: