Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Removing the disconnect-on-warning #1072

Closed
TheBlueMatt opened this issue Apr 29, 2023 · 3 comments · Fixed by #1075
Closed

Removing the disconnect-on-warning #1072

TheBlueMatt opened this issue Apr 29, 2023 · 3 comments · Fixed by #1075

Comments

@TheBlueMatt
Copy link
Collaborator

I saw some users complaining that their lnd node is disconnecting a CLN node because the CLN node sent a warning "too many channel announcements with funding tx already spent, please check your bitcoin node". While maybe the underlying issue should be fixed, ISTM we should remove the "a recipient of a warning MAY disconnect" thing. The sender of the warning has more context about whether its a big deal or not, so we should leave it up to them to disconnect or not.

@TheBlueMatt
Copy link
Collaborator Author

I apologize, apparently that warning is sent by an older version of eclair, either way my comment stands.

@vincenzopalazzo
Copy link
Contributor

vincenzopalazzo commented Apr 30, 2023

a recipient of a warning MAY disconnect

yes I agree there is no point to have a warning if we do disconnect, it is an error at this point, IMHO the warning should not impact the node connection in any case (maybe?)

@t-bast
Copy link
Collaborator

t-bast commented May 2, 2023

I agree that we should remove any disconnect-on-warning requirements.
So in this specific case, I guess lnd should then update their behavior to avoid disconnection when receiving the warning.
FWIW, eclair doesn't disconnect when receiving a warning.

t-bast added a commit to t-bast/bolts that referenced this issue May 5, 2023
We generally shouldn't disconnect when sending or receiving warning
messages. Whenever disconnecting after a warning makes sense, it should
be specified in the requirements linked to that specific scenario.

Fixes lightning#1072
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants