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.

Sign up for GitHub

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

Jump to bottom

TemporaryChannelFailure without channel update #5822

Closed
t-bast opened this issue Dec 14, 2022 · 2 comments
Closed

TemporaryChannelFailure without channel update #5822

t-bast opened this issue Dec 14, 2022 · 2 comments
Assignees
@rustyrussell
Labels
protocol These issues are protocol level issues that should be discussed on the protocol spec repo

Comments

@t-bast
Copy link

t-bast commented Dec 14, 2022 •
edited
Loading

I'm making some e2e tests with cln 22.11, and noticed that we fail to parse some of your errors.
The reason is that cln sometimes returns a TemporaryChannelFailure without an enclosed channel_update, while we expect one. For example, I'm receiving this error (after decryption):

86abd17911a15dcd41053173c1cbaf9d173bb7199e170c518127ff97c2cbbdb8 (mac)
0004 (failure message length)
1007 0000 (temporary_channel_failure with channel_update length = 0)
00fc (padding length)
000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 (padding)

The spec says:

Please note that the channel_update field is mandatory in messages whose failure_code includes the UPDATE flag.

We've had this issue in the past with lnd, but they eventually fixed it.

This is creating real issues for payment reliability, because this prevents retries with smaller amounts since we don't realize that this is a temporary failure (which most likely indicates a liquidity issue).

We should decide whether we want to relax the spec and allow not enclosing a channel_update in this case, or if this is a bug that must be fixed when creating the error.
@vincenzopalazzo vincenzopalazzo added the protocol These issues are protocol level issues that should be discussed on the protocol spec repo label Dec 14, 2022
@rustyrussell rustyrussell added this to the v23.02 milestone Dec 22, 2022
@rustyrussell rustyrussell self-assigned this Dec 22, 2022
@rustyrussell rustyrussell modified the milestones: v23.02, v23.05 Feb 6, 2023
@ShahanaFarooqui ShahanaFarooqui modified the milestones: v23.05, v23.08 Apr 5, 2023
@rustyrussell
Copy link
Contributor

OK, note that we currently do this on unannounced channels. We will start in next release generating an appropriate bogus one for these.

@rustyrussell rustyrussell modified the milestones: v23.08, v23.11 Jul 28, 2023
rustyrussell added a commit to rustyrussell/lightning that referenced this issue Jul 28, 2023
@rustyrussell
gossipd: tell lightningd about all our previous channel_updates at st…
83a6d67 
…artup.

This will at least *help* the case where these were not populated, causing us
to send errors without channel_updated appended.

It's not perfect: we can still send such errors if the gossip store is
corrupted, and we still send them for private channels, but it should
help.

(The much better fix is far more invasive, so slips to next release!)

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: ElementsProject#5822
@rustyrussell rustyrussell mentioned this issue Jul 28, 2023
Merged
@nepet nepet removed this from the v23.11 milestone Dec 3, 2023
@t-bast
Copy link
Author

t-bast commented Jun 17, 2024

We don't recommend including the channel_update anymore, so closing this PR (lightning/bolts#1173).

@t-bast t-bast closed this as completed Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rustyrussell rustyrussell

Labels
protocol These issues are protocol level issues that should be discussed on the protocol spec repo
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@rustyrussell @nepet @vincenzopalazzo @ShahanaFarooqui @t-bast