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

subd: Do not send feerate updates to non-channeld subds #6937

Merged

Conversation

cdecker
Copy link
Member

@cdecker cdecker commented Dec 12, 2023

Turns out we were sending feerate updates to daemons that do not understand it. Don't do that!

Closes #6932

Changelog-Fixed: channeld: We could crash closingd by sending it a channeld message

Turns out we were sending feerate updates to daemons that do not
understand it. Don't do that!

Closes ElementsProject#6932

Changelog-Fixed: channeld: We could crash `closingd` by sending it a `channeld` message
@cdecker cdecker marked this pull request as ready for review December 12, 2023 15:27
@cdecker cdecker merged commit a9aa93d into ElementsProject:master Dec 12, 2023
38 checks passed
@whitslack
Copy link
Collaborator

whitslack commented Dec 12, 2023

@cdecker: I ran into this today too. Wouldn't a cleaner fix be to drop CLOSINGD_SIGEXCHANGE from the set of channel_states for which channel_state_fees_can_change returns true?

https://github.com/ElementsProject/lightning/blob/v23.11/lightningd/channel.h#L518

Then the !channel_state_fees_can_change(channel->state) check at the top of try_update_feerates(…) would fail early.


Addendum: Note that in v23.05.2, try_update_feerates would test channel_fees_can_change, which would return true only if the channel state was CHANNELD_NORMAL or CHANNELD_SHUTTING_DOWN. But then 0b4622b replaced channel_fees_can_change by channel_state_fees_can_change and inexplicably added CLOSINGD_SIGEXCHANGE as a state in which fees can change.

gentoo-repo-qa-bot pushed a commit to gentoo-mirror/bitcoin that referenced this pull request Dec 12, 2023
This is a fix for a crash bug and thus is deemed critical.

See: ElementsProject/lightning#6937
@rustyrussell rustyrussell added this to the v23.11.1 milestone Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash in lightningd
3 participants