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

lightningd: don't force state if gossipd gives us an unexpected channel_update. #7058

Merged

Conversation

rustyrussell
Copy link
Contributor

This was triggered by the recover plugin tests (not yet merged!) and causes a crash because we don't have signatures yet. It can only happen if we lost our database, but at least don't crash!

Fixes the crash in #6853

@rustyrussell rustyrussell added this to the v24.02 milestone Feb 9, 2024
@cdecker cdecker force-pushed the fix-lost-db-gossip-crash branch 2 times, most recently from 02909df to 9cfc3a3 Compare February 9, 2024 13:52
@cdecker
Copy link
Member

cdecker commented Feb 9, 2024

The new warning is being hit in several places:

lightningd-1 2024-02-09T14:34:17.640Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-chan#1: gossipd gave us channel_update for channel in gossip_state CGOSSIP_NOT_DEEP_ENOUGH

It being BROKEN means it kill CI. Shall we address the places this happens or should we downgrade the severity, and fix up in the background? It's useful information to have for debugging, so I'm tempted to go with the severity reduction.

@cdecker cdecker force-pushed the fix-lost-db-gossip-crash branch 3 times, most recently from 33af030 to 8e04c66 Compare February 13, 2024 14:48
@cdecker
Copy link
Member

cdecker commented Feb 13, 2024

Removing from the milestone as it still fails with a **BROKEN** message. If it gets fixed in time for the later RCs we might consider it.

@cdecker cdecker removed this from the v24.02 milestone Feb 13, 2024
@rustyrussell rustyrussell force-pushed the fix-lost-db-gossip-crash branch from 8e04c66 to a1377c8 Compare February 14, 2024 01:11
…el_update.

This was triggered by the recover plugin tests (not yet merged!) and causes a crash
because we don't have signatures yet.  It can only happen if we lost our database,
but at least don't crash!

Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell rustyrussell force-pushed the fix-lost-db-gossip-crash branch from a1377c8 to 62bc22d Compare February 14, 2024 01:11
@rustyrussell
Copy link
Contributor Author

This was overzealous: fixed and tested. Since it's an actual crash, I'll apply once CI is happy...

@rustyrussell rustyrussell merged commit d716e6d into ElementsProject:master Feb 14, 2024
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants