Skip to content
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

common: downgrade "internal error" errors from lnd. #5326

Merged

Conversation

rustyrussell
Copy link
Contributor

@rustyrussell rustyrussell commented Jun 20, 2022

Prior to 0.11.0 we had cases where we would treat errors
as warnings: regretfully, this is still needed. This message
in particular has been widely reported, and it now causes
channel force closes.

Downgrade and log. I did insert some snarky log message earlier,
but hey, I'm sure CLN has done worse things to our peers!

Fixes: #5102
Changelog-None: Changelog is updated in v0.11.2 branch, which will be merged after that is released.

Prior to 0.11.0 we had cases where we would treat errors
as warnings: regretfully, this is still needed.  This message
in particular has been widely reported, and it now causes
channel force closes.

Downgrade and log.  I did insert some snarky log message earlier,
but hey, I'm sure CLN has done worse things to our peers!

Signed-off-by: Rusty Russell <[email protected]>
Changelog-Fixed: Protocol: treat LND "internal error" as warnings, not force close events (as we did in v0.10).
@whitslack
Copy link
Collaborator

I'm running my node with this patch now.

Copy link
Collaborator

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

utACK ae30fb7

@cdecker
Copy link
Member

cdecker commented Jun 21, 2022

ACK ae30fb7

Thanks to @zerofeerouting for another report.

"desc" here is the sanitized message, eg:
"ERROR error channel 0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef: internal error"

Signed-off-by: Rusty Russell <[email protected]>
Copy link
Collaborator

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

utACK effbd80

worried if you can write a lnprototest for this? if you can point out on how to find the lnd error message I can write a test on the flight I think? or I'm simplify the stuff to much?

@rustyrussell
Copy link
Contributor Author

rustyrussell commented Jun 24, 2022

I'm running my node with this patch now.

@whitslack Please note original patch was ineffective! Second commit fixes properly.

@rustyrussell
Copy link
Contributor Author

utACK effbd80

worried if you can write a lnprototest for this? if you can point out on how to find the lnd error message I can write a test on the flight I think? or I'm simplify the stuff to much?

I don't think workarounds on (hopefully temporary!) implementation bugs should be included in lnprototest.

@rustyrussell rustyrussell merged commit a196c77 into ElementsProject:master Jun 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

force close on peer with "internal error" no further explanations
4 participants